Skip to content

Commit

Permalink
Made external permissioned users and slack users show diff (#3147)
Browse files Browse the repository at this point in the history
* Made external permissioned users and slack users show diff

* finished

* Fix typing

* k

* Fix

* k

---------

Co-authored-by: Weves <chrisweaver101@gmail.com>
  • Loading branch information
hagen-danswer and Weves authored Nov 17, 2024
1 parent 521425a commit 954b5b2
Show file tree
Hide file tree
Showing 13 changed files with 288 additions and 132 deletions.
42 changes: 42 additions & 0 deletions backend/alembic/versions/dfbe9e93d3c7_extended_role_for_non_web.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
"""extended_role_for_non_web
Revision ID: dfbe9e93d3c7
Revises: 9cf5c00f72fe
Create Date: 2024-11-16 07:54:18.727906
"""
from alembic import op
import sqlalchemy as sa

# revision identifiers, used by Alembic.
revision = "dfbe9e93d3c7"
down_revision = "9cf5c00f72fe"
branch_labels = None
depends_on = None


def upgrade() -> None:
op.execute(
"""
UPDATE "user"
SET role = 'EXT_PERM_USER'
WHERE has_web_login = false
"""
)
op.drop_column("user", "has_web_login")


def downgrade() -> None:
op.add_column(
"user",
sa.Column("has_web_login", sa.Boolean(), nullable=False, server_default="true"),
)

op.execute(
"""
UPDATE "user"
SET has_web_login = false,
role = 'BASIC'
WHERE role IN ('SLACK_USER', 'EXT_PERM_USER')
"""
)
13 changes: 11 additions & 2 deletions backend/danswer/auth/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,24 @@ class UserRole(str, Enum):
groups they are curators of
- Global Curator can perform admin actions
for all groups they are a member of
- Limited can access a limited set of basic api endpoints
- Slack are users that have used danswer via slack but dont have a web login
- External permissioned users that have been picked up during the external permissions sync process but don't have a web login
"""

LIMITED = "limited"
BASIC = "basic"
ADMIN = "admin"
CURATOR = "curator"
GLOBAL_CURATOR = "global_curator"
SLACK_USER = "slack_user"
EXT_PERM_USER = "ext_perm_user"

def is_web_login(self) -> bool:
return self not in [
UserRole.SLACK_USER,
UserRole.EXT_PERM_USER,
]


class UserStatus(str, Enum):
Expand All @@ -34,10 +45,8 @@ class UserRead(schemas.BaseUser[uuid.UUID]):

class UserCreate(schemas.BaseUserCreate):
role: UserRole = UserRole.BASIC
has_web_login: bool | None = True
tenant_id: str | None = None


class UserUpdate(schemas.BaseUserUpdate):
role: UserRole
has_web_login: bool | None = True
39 changes: 20 additions & 19 deletions backend/danswer/auth/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
from httpx_oauth.oauth2 import OAuth2Token
from pydantic import BaseModel
from sqlalchemy import text
from sqlalchemy.orm import attributes
from sqlalchemy.orm import Session

from danswer.auth.api_key import get_hashed_api_key_from_request
Expand Down Expand Up @@ -222,6 +221,8 @@ class UserManager(UUIDIDMixin, BaseUserManager[User, uuid.UUID]):
reset_password_token_secret = USER_AUTH_SECRET
verification_token_secret = USER_AUTH_SECRET

user_db: SQLAlchemyUserDatabase[User, uuid.UUID]

async def create(
self,
user_create: schemas.UC | UserCreate,
Expand All @@ -247,7 +248,9 @@ async def create(
verify_email_is_invited(user_create.email)
verify_email_domain(user_create.email)
if MULTI_TENANT:
tenant_user_db = SQLAlchemyUserAdminDB(db_session, User, OAuthAccount)
tenant_user_db = SQLAlchemyUserAdminDB[User, uuid.UUID](
db_session, User, OAuthAccount
)
self.user_db = tenant_user_db
self.database = tenant_user_db

Expand All @@ -266,14 +269,9 @@ async def create(
except exceptions.UserAlreadyExists:
user = await self.get_by_email(user_create.email)
# Handle case where user has used product outside of web and is now creating an account through web
if (
not user.has_web_login
and hasattr(user_create, "has_web_login")
and user_create.has_web_login
):
if not user.role.is_web_login() and user_create.role.is_web_login():
user_update = UserUpdate(
password=user_create.password,
has_web_login=True,
role=user_create.role,
is_verified=user_create.is_verified,
)
Expand All @@ -287,7 +285,7 @@ async def create(
return user

async def oauth_callback(
self: "BaseUserManager[models.UOAP, models.ID]",
self,
oauth_name: str,
access_token: str,
account_id: str,
Expand All @@ -298,7 +296,7 @@ async def oauth_callback(
*,
associate_by_email: bool = False,
is_verified_by_default: bool = False,
) -> models.UOAP:
) -> User:
referral_source = None
if request:
referral_source = getattr(request.state, "referral_source", None)
Expand All @@ -324,9 +322,11 @@ async def oauth_callback(
verify_email_domain(account_email)

if MULTI_TENANT:
tenant_user_db = SQLAlchemyUserAdminDB(db_session, User, OAuthAccount)
tenant_user_db = SQLAlchemyUserAdminDB[User, uuid.UUID](
db_session, User, OAuthAccount
)
self.user_db = tenant_user_db
self.database = tenant_user_db # type: ignore
self.database = tenant_user_db

oauth_account_dict = {
"oauth_name": oauth_name,
Expand Down Expand Up @@ -378,7 +378,11 @@ async def oauth_callback(
and existing_oauth_account.oauth_name == oauth_name
):
user = await self.user_db.update_oauth_account(
user, existing_oauth_account, oauth_account_dict
user,
# NOTE: OAuthAccount DOES implement the OAuthAccountProtocol
# but the type checker doesn't know that :(
existing_oauth_account, # type: ignore
oauth_account_dict,
)

# NOTE: Most IdPs have very short expiry times, and we don't want to force the user to
Expand All @@ -391,16 +395,15 @@ async def oauth_callback(
)

# Handle case where user has used product outside of web and is now creating an account through web
if not user.has_web_login: # type: ignore
if not user.role.is_web_login():
await self.user_db.update(
user,
{
"is_verified": is_verified_by_default,
"has_web_login": True,
"role": UserRole.BASIC,
},
)
user.is_verified = is_verified_by_default
user.has_web_login = True # type: ignore

# this is needed if an organization goes from `TRACK_EXTERNAL_IDP_EXPIRY=true` to `false`
# otherwise, the oidc expiry will always be old, and the user will never be able to login
Expand Down Expand Up @@ -475,9 +478,7 @@ async def authenticate(
self.password_helper.hash(credentials.password)
return None

has_web_login = attributes.get_attribute(user, "has_web_login")

if not has_web_login:
if not user.role.is_web_login():
raise BasicAuthenticationError(
detail="NO_WEB_LOGIN_AND_HAS_NO_PASSWORD",
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from danswer.db.enums import AccessType
from danswer.db.enums import ConnectorCredentialPairStatus
from danswer.db.models import ConnectorCredentialPair
from danswer.db.users import batch_add_non_web_user_if_not_exists
from danswer.db.users import batch_add_ext_perm_user_if_not_exists
from danswer.redis.redis_connector import RedisConnector
from danswer.redis.redis_connector_doc_perm_sync import (
RedisConnectorPermissionSyncData,
Expand Down Expand Up @@ -301,7 +301,7 @@ def update_external_document_permissions_task(
try:
with get_session_with_tenant(tenant_id) as db_session:
# Then we build the update requests to update vespa
batch_add_non_web_user_if_not_exists(
batch_add_ext_perm_user_if_not_exists(
db_session=db_session,
emails=list(external_access.external_user_emails),
)
Expand Down
4 changes: 2 additions & 2 deletions backend/danswer/danswerbot/slack/handlers/handle_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from danswer.danswerbot.slack.utils import update_emote_react
from danswer.db.engine import get_session_with_tenant
from danswer.db.models import SlackBotConfig
from danswer.db.users import add_non_web_user_if_not_exists
from danswer.db.users import add_slack_user_if_not_exists
from danswer.utils.logger import setup_logger
from shared_configs.configs import SLACK_CHANNEL_ID

Expand Down Expand Up @@ -213,7 +213,7 @@ def handle_message(

with get_session_with_tenant(tenant_id) as db_session:
if message_info.email:
add_non_web_user_if_not_exists(db_session, message_info.email)
add_slack_user_if_not_exists(db_session, message_info.email)

# first check if we need to respond with a standard answer
used_standard_answer = handle_standard_answers(
Expand Down
8 changes: 6 additions & 2 deletions backend/danswer/db/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from typing import Dict

from fastapi import Depends
from fastapi_users.models import ID
from fastapi_users.models import UP
from fastapi_users_db_sqlalchemy import SQLAlchemyUserDatabase
from fastapi_users_db_sqlalchemy.access_token import SQLAlchemyAccessTokenDatabase
Expand Down Expand Up @@ -43,7 +44,10 @@ def get_total_users_count(db_session: Session) -> int:
"""
user_count = (
db_session.query(User)
.filter(~User.email.endswith(get_api_key_email_pattern())) # type: ignore
.filter(
~User.email.endswith(get_api_key_email_pattern()), # type: ignore
User.role != UserRole.EXT_PERM_USER,
)
.count()
)
invited_users = len(get_invited_users())
Expand All @@ -61,7 +65,7 @@ async def get_user_count() -> int:


# Need to override this because FastAPI Users doesn't give flexibility for backend field creation logic in OAuth flow
class SQLAlchemyUserAdminDB(SQLAlchemyUserDatabase):
class SQLAlchemyUserAdminDB(SQLAlchemyUserDatabase[UP, ID]):
async def create(
self,
create_dict: Dict[str, Any],
Expand Down
2 changes: 0 additions & 2 deletions backend/danswer/db/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,6 @@ class User(SQLAlchemyBaseUserTableUUID, Base):
notifications: Mapped[list["Notification"]] = relationship(
"Notification", back_populates="user"
)
# Whether the user has logged in via web. False if user has only used Danswer through Slack bot
has_web_login: Mapped[bool] = mapped_column(Boolean, default=True)
cc_pairs: Mapped[list["ConnectorCredentialPair"]] = relationship(
"ConnectorCredentialPair",
back_populates="creator",
Expand Down
Loading

0 comments on commit 954b5b2

Please sign in to comment.