diff --git a/backend/alembic/versions/dfbe9e93d3c7_extended_role_for_non_web.py b/backend/alembic/versions/dfbe9e93d3c7_extended_role_for_non_web.py new file mode 100644 index 00000000000..3f717ff09f1 --- /dev/null +++ b/backend/alembic/versions/dfbe9e93d3c7_extended_role_for_non_web.py @@ -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') + """ + ) diff --git a/backend/danswer/auth/schemas.py b/backend/danswer/auth/schemas.py index ce503afe76e..e943504c584 100644 --- a/backend/danswer/auth/schemas.py +++ b/backend/danswer/auth/schemas.py @@ -13,6 +13,9 @@ 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" @@ -20,6 +23,14 @@ class UserRole(str, Enum): 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): @@ -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 diff --git a/backend/danswer/auth/users.py b/backend/danswer/auth/users.py index 530b7f48ac2..5fd7d0cae95 100644 --- a/backend/danswer/auth/users.py +++ b/backend/danswer/auth/users.py @@ -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 @@ -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, @@ -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 @@ -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, ) @@ -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, @@ -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) @@ -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, @@ -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 @@ -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 @@ -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", ) diff --git a/backend/danswer/background/celery/tasks/doc_permission_syncing/tasks.py b/backend/danswer/background/celery/tasks/doc_permission_syncing/tasks.py index e4a715f425a..6a5761a7428 100644 --- a/backend/danswer/background/celery/tasks/doc_permission_syncing/tasks.py +++ b/backend/danswer/background/celery/tasks/doc_permission_syncing/tasks.py @@ -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, @@ -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), ) diff --git a/backend/danswer/danswerbot/slack/handlers/handle_message.py b/backend/danswer/danswerbot/slack/handlers/handle_message.py index ffbe902c5ec..8ed47ffcb7c 100644 --- a/backend/danswer/danswerbot/slack/handlers/handle_message.py +++ b/backend/danswer/danswerbot/slack/handlers/handle_message.py @@ -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 @@ -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( diff --git a/backend/danswer/db/auth.py b/backend/danswer/db/auth.py index 95d15b259bd..bc4047109fa 100644 --- a/backend/danswer/db/auth.py +++ b/backend/danswer/db/auth.py @@ -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 @@ -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()) @@ -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], diff --git a/backend/danswer/db/models.py b/backend/danswer/db/models.py index 4f7667b7a7a..5f47eade150 100644 --- a/backend/danswer/db/models.py +++ b/backend/danswer/db/models.py @@ -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", diff --git a/backend/danswer/db/users.py b/backend/danswer/db/users.py index 2c173f04443..24f7dc501c7 100644 --- a/backend/danswer/db/users.py +++ b/backend/danswer/db/users.py @@ -1,6 +1,7 @@ from collections.abc import Sequence from uuid import UUID +from fastapi import HTTPException from fastapi_users.password import PasswordHelper from sqlalchemy import func from sqlalchemy import select @@ -10,15 +11,94 @@ from danswer.db.models import User +def validate_user_role_update(requested_role: UserRole, current_role: UserRole) -> None: + """ + Validate that a user role update is valid. + Assumed only admins can hit this endpoint. + raise if: + - requested role is a curator + - requested role is a slack user + - requested role is an external permissioned user + - requested role is a limited user + - current role is a slack user + - current role is an external permissioned user + - current role is a limited user + """ + + if current_role == UserRole.SLACK_USER: + raise HTTPException( + status_code=400, + detail="To change a Slack User's role, they must first login to Danswer via the web app.", + ) + + if current_role == UserRole.EXT_PERM_USER: + # This shouldn't happen, but just in case + raise HTTPException( + status_code=400, + detail="To change an External Permissioned User's role, they must first login to Danswer via the web app.", + ) + + if current_role == UserRole.LIMITED: + raise HTTPException( + status_code=400, + detail="To change a Limited User's role, they must first login to Danswer via the web app.", + ) + + if requested_role == UserRole.CURATOR: + # This shouldn't happen, but just in case + raise HTTPException( + status_code=400, + detail="Curator role must be set via the User Group Menu", + ) + + if requested_role == UserRole.LIMITED: + # This shouldn't happen, but just in case + raise HTTPException( + status_code=400, + detail=( + "A user cannot be set to a Limited User role. " + "This role is automatically assigned to users through certain endpoints in the API." + ), + ) + + if requested_role == UserRole.SLACK_USER: + # This shouldn't happen, but just in case + raise HTTPException( + status_code=400, + detail=( + "A user cannot be set to a Slack User role. " + "This role is automatically assigned to users who only use Danswer via Slack." + ), + ) + + if requested_role == UserRole.EXT_PERM_USER: + # This shouldn't happen, but just in case + raise HTTPException( + status_code=400, + detail=( + "A user cannot be set to an External Permissioned User role. " + "This role is automatically assigned to users who have been " + "pulled in to the system via an external permissions system." + ), + ) + + def list_users( - db_session: Session, email_filter_string: str = "", user: User | None = None + db_session: Session, email_filter_string: str = "", include_external: bool = False ) -> Sequence[User]: """List all users. No pagination as of now, as the # of users is assumed to be relatively small (<< 1 million)""" stmt = select(User) + where_clause = [] + + if not include_external: + where_clause.append(User.role != UserRole.EXT_PERM_USER) + if email_filter_string: - stmt = stmt.where(User.email.ilike(f"%{email_filter_string}%")) # type: ignore + where_clause.append(User.email.ilike(f"%{email_filter_string}%")) # type: ignore + + stmt = stmt.where(*where_clause) return db_session.scalars(stmt).unique().all() @@ -45,68 +125,56 @@ def get_user_by_email(email: str, db_session: Session) -> User | None: def fetch_user_by_id(db_session: Session, user_id: UUID) -> User | None: - user = db_session.query(User).filter(User.id == user_id).first() # type: ignore - - return user + return db_session.query(User).filter(User.id == user_id).first() # type: ignore -def _generate_non_web_user(email: str) -> User: +def _generate_non_web_slack_user(email: str) -> User: fastapi_users_pw_helper = PasswordHelper() password = fastapi_users_pw_helper.generate() hashed_pass = fastapi_users_pw_helper.hash(password) return User( email=email, hashed_password=hashed_pass, - has_web_login=False, - role=UserRole.BASIC, + role=UserRole.SLACK_USER, ) -def add_non_web_user_if_not_exists(db_session: Session, email: str) -> User: +def add_slack_user_if_not_exists(db_session: Session, email: str) -> User: + email = email.lower() user = get_user_by_email(email, db_session) if user is not None: + # If the user is an external permissioned user, we update it to a slack user + if user.role == UserRole.EXT_PERM_USER: + user.role = UserRole.SLACK_USER + db_session.commit() return user - user = _generate_non_web_user(email=email) + user = _generate_non_web_slack_user(email=email) db_session.add(user) db_session.commit() return user -def add_non_web_user_if_not_exists__no_commit(db_session: Session, email: str) -> User: - user = get_user_by_email(email, db_session) - if user is not None: - return user - - user = _generate_non_web_user(email=email) - db_session.add(user) - db_session.flush() # generate id - return user - - -def batch_add_non_web_user_if_not_exists__no_commit( - db_session: Session, emails: list[str] -) -> list[User]: - found_users, missing_user_emails = get_users_by_emails(db_session, emails) - - new_users: list[User] = [] - for email in missing_user_emails: - new_users.append(_generate_non_web_user(email=email)) - - db_session.add_all(new_users) - db_session.flush() # generate ids - - return found_users + new_users +def _generate_non_web_permissioned_user(email: str) -> User: + fastapi_users_pw_helper = PasswordHelper() + password = fastapi_users_pw_helper.generate() + hashed_pass = fastapi_users_pw_helper.hash(password) + return User( + email=email, + hashed_password=hashed_pass, + role=UserRole.EXT_PERM_USER, + ) -def batch_add_non_web_user_if_not_exists( +def batch_add_ext_perm_user_if_not_exists( db_session: Session, emails: list[str] ) -> list[User]: + emails = [email.lower() for email in emails] found_users, missing_user_emails = get_users_by_emails(db_session, emails) new_users: list[User] = [] for email in missing_user_emails: - new_users.append(_generate_non_web_user(email=email)) + new_users.append(_generate_non_web_permissioned_user(email=email)) db_session.add_all(new_users) db_session.commit() diff --git a/backend/danswer/server/manage/users.py b/backend/danswer/server/manage/users.py index 46832e58f04..4fe2b0fbcc0 100644 --- a/backend/danswer/server/manage/users.py +++ b/backend/danswer/server/manage/users.py @@ -49,6 +49,7 @@ from danswer.db.models import User__UserGroup from danswer.db.users import get_user_by_email from danswer.db.users import list_users +from danswer.db.users import validate_user_role_update from danswer.key_value_store.factory import get_kv_store from danswer.server.manage.models import AllUsersResponse from danswer.server.manage.models import UserByEmail @@ -84,28 +85,31 @@ def set_user_role( if not user_to_update: raise HTTPException(status_code=404, detail="User not found") - if user_role_update_request.new_role == UserRole.CURATOR: - raise HTTPException( - status_code=400, - detail="Curator role must be set via the User Group Menu", - ) - - if user_to_update.role == user_role_update_request.new_role: + current_role = user_to_update.role + requested_role = user_role_update_request.new_role + if requested_role == current_role: return - if current_user.id == user_to_update.id: + # This will raise an exception if the role update is invalid + validate_user_role_update( + requested_role=requested_role, + current_role=current_role, + ) + + if user_to_update.id == current_user.id: raise HTTPException( status_code=400, detail="An admin cannot demote themselves from admin role!", ) - if user_to_update.role == UserRole.CURATOR: + if requested_role == UserRole.CURATOR: + # Remove all curator db relationships before changing role fetch_ee_implementation_or_noop( "danswer.db.user_group", "remove_curator_status__no_commit", )(db_session, user_to_update) - user_to_update.role = user_role_update_request.new_role.value + user_to_update.role = user_role_update_request.new_role db_session.commit() @@ -123,7 +127,7 @@ def list_all_users( users = [ user - for user in list_users(db_session, email_filter_string=q, user=user) + for user in list_users(db_session, email_filter_string=q) if not is_api_key_email_address(user.email) ] accepted_emails = {user.email for user in users} diff --git a/backend/ee/danswer/db/external_perm.py b/backend/ee/danswer/db/external_perm.py index 1ccb30db5e2..5411d3c8d34 100644 --- a/backend/ee/danswer/db/external_perm.py +++ b/backend/ee/danswer/db/external_perm.py @@ -9,7 +9,7 @@ from danswer.access.utils import prefix_group_w_source from danswer.configs.constants import DocumentSource from danswer.db.models import User__ExternalUserGroupId -from danswer.db.users import batch_add_non_web_user_if_not_exists__no_commit +from danswer.db.users import batch_add_ext_perm_user_if_not_exists class ExternalUserGroup(BaseModel): @@ -49,10 +49,6 @@ def replace_user__ext_group_for_cc_pair( This function clears all existing external user group relations for a given cc_pair_id and replaces them with the new group definitions and commits the changes. """ - delete_user__ext_group_for_cc_pair__no_commit( - db_session=db_session, - cc_pair_id=cc_pair_id, - ) # collect all emails from all groups to batch add all users at once for efficiency all_group_member_emails = set() @@ -61,10 +57,15 @@ def replace_user__ext_group_for_cc_pair( all_group_member_emails.add(user_email) # batch add users if they don't exist and get their ids - all_group_members = batch_add_non_web_user_if_not_exists__no_commit( + all_group_members = batch_add_ext_perm_user_if_not_exists( db_session=db_session, emails=list(all_group_member_emails) ) + delete_user__ext_group_for_cc_pair__no_commit( + db_session=db_session, + cc_pair_id=cc_pair_id, + ) + # map emails to ids email_id_map = {user.email: user.id for user in all_group_members} diff --git a/backend/ee/danswer/server/saml.py b/backend/ee/danswer/server/saml.py index 7ff385d1377..81a7b5d8d9c 100644 --- a/backend/ee/danswer/server/saml.py +++ b/backend/ee/danswer/server/saml.py @@ -64,7 +64,6 @@ async def upsert_saml_user(email: str) -> User: password=hashed_pass, is_verified=True, role=role, - has_web_login=True, ) ) diff --git a/web/src/components/admin/users/SignedUpUserTable.tsx b/web/src/components/admin/users/SignedUpUserTable.tsx index dc7da5113b3..caae9383a6e 100644 --- a/web/src/components/admin/users/SignedUpUserTable.tsx +++ b/web/src/components/admin/users/SignedUpUserTable.tsx @@ -1,4 +1,10 @@ -import { type User, UserStatus, UserRole, USER_ROLE_LABELS } from "@/lib/types"; +import { + type User, + UserStatus, + UserRole, + USER_ROLE_LABELS, + INVALID_ROLE_HOVER_TEXT, +} from "@/lib/types"; import CenteredPageSelector from "./CenteredPageSelector"; import { type PageSelectorProps } from "@/components/PageSelector"; import { HidableSection } from "@/app/admin/assistants/HidableSection"; @@ -87,27 +93,34 @@ const UserRoleDropdown = ({ - {Object.entries(USER_ROLE_LABELS).map(([role, label]) => - !isPaidEnterpriseFeaturesEnabled && - (role === UserRole.CURATOR || - role === UserRole.GLOBAL_CURATOR) ? null : ( - - {label} - - ) + {(Object.entries(USER_ROLE_LABELS) as [UserRole, string][]).map( + ([role, label]) => { + // Dont want to ever show external permissioned users because it's scary + if (role === UserRole.EXT_PERM_USER) return null; + + // Only want to show limited users if paid enterprise features are enabled + // Also, dont want to show these other roles in general + const isNotVisibleRole = + (!isPaidEnterpriseFeaturesEnabled && + role === UserRole.GLOBAL_CURATOR) || + role === UserRole.CURATOR || + role === UserRole.LIMITED || + role === UserRole.SLACK_USER; + + // Always show the current role + const isCurrentRole = user.role === role; + + return isNotVisibleRole && !isCurrentRole ? null : ( + + {label} + + ); + } )} @@ -268,38 +281,41 @@ const SignedUpUserTable = ({ - {users.map((user) => ( - - {user.email} - - - - - {user.status === "live" ? "Active" : "Inactive"} - - -
- user.role !== UserRole.EXT_PERM_USER) + .map((user) => ( + + {user.email} + + - {user.status == UserStatus.deactivated && ( - + + {user.status === "live" ? "Active" : "Inactive"} + + +
+ - )} -
-
-
- ))} + {user.status == UserStatus.deactivated && ( + + )} +
+
+
+ ))}
diff --git a/web/src/lib/types.ts b/web/src/lib/types.ts index eff53f24d36..c156218257b 100644 --- a/web/src/lib/types.ts +++ b/web/src/lib/types.ts @@ -23,14 +23,28 @@ export enum UserRole { ADMIN = "admin", CURATOR = "curator", GLOBAL_CURATOR = "global_curator", + EXT_PERM_USER = "ext_perm_user", + SLACK_USER = "slack_user", } export const USER_ROLE_LABELS: Record = { - [UserRole.LIMITED]: "Limited", [UserRole.BASIC]: "Basic", [UserRole.ADMIN]: "Admin", [UserRole.GLOBAL_CURATOR]: "Global Curator", [UserRole.CURATOR]: "Curator", + [UserRole.LIMITED]: "Limited", + [UserRole.EXT_PERM_USER]: "External Permissioned User", + [UserRole.SLACK_USER]: "Slack User", +}; + +export const INVALID_ROLE_HOVER_TEXT: Partial> = { + [UserRole.BASIC]: "Basic users can't perform any admin actions", + [UserRole.ADMIN]: "Admin users can perform all admin actions", + [UserRole.GLOBAL_CURATOR]: + "Global Curator users can perform admin actions for all groups they are a member of", + [UserRole.CURATOR]: "Curator role must be assigned in the Groups tab", + [UserRole.SLACK_USER]: + "This role is automatically assigned to users who only use Danswer via Slack", }; export interface User {