Add user when they interact outside of UI (e.g. Slack bot) #2369
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| def upgrade() -> None: | ||
| op.add_column( | ||
| "user", | ||
| sa.Column("has_web_login", sa.Boolean(), nullable=False, server_default="true"), |
There was a problem hiding this comment.
We should test with another user as well (turn off email verification and use basic auth). Do a login with [email protected] off of main and then upgrade the version. I think it will fail due to the NOT NULL constraint. We have to support graceful upgrades
backend/danswer/auth/schemas.py
Outdated
| @@ -1,5 +1,6 @@ | |||
| import uuid | |||
| from enum import Enum | |||
| from typing import Optional | |||
There was a problem hiding this comment.
Is there a reason we can't use the built in typing that came with the more recent python versions?
| @@ -47,6 +52,25 @@ def send_msg_ack_to_user(details: SlackMessageInfo, client: WebClient) -> None: | |||
| ) | |||
|
|
|||
|
|
|||
| def add_user_if_not_exists(email: str, db_session: Session) -> User: | |||
There was a problem hiding this comment.
We like to throw these functions that interface with the DB models (and that may be reuseable) in the related file under backend/db. Makes organization/finding things easy and helps prevents circular imports.
| @@ -208,7 +232,14 @@ def handle_message( | |||
| except SlackApiError as e: | |||
| logger.error(f"Was not able to react to user message due to: {e}") | |||
|
|
|||
| slack_user_info = expert_info_from_slack_id( | |||
There was a problem hiding this comment.
Since we've added this in a few places recently, is there a way we can do it at the top level and pass it through? That way we make less Slack calls. In this case the volume shouldn't be very high but I think we can probably make this small optimization. The case where it would matter is if for example they have a Slack connector that has been rate limited due to volume of requests and it's affecting this side as well (some folks don't create separate Slack apps for connector and SlackBot)
…i#2369) * Add user when they interact outside of UI (e.g. Slack bot) * fix mypy errors * don't use user manager to avoid async messiness * fix email is none scenario * fix mypy * make code slightly clearer * PR comments * get slack email in generate button as well * fix alembic migration * update name to be more descriptive --------- Co-authored-by: Hyeong Joon Suh <[email protected]>
…i#2369) * Add user when they interact outside of UI (e.g. Slack bot) * fix mypy errors * don't use user manager to avoid async messiness * fix email is none scenario * fix mypy * make code slightly clearer * PR comments * get slack email in generate button as well * fix alembic migration * update name to be more descriptive --------- Co-authored-by: Hyeong Joon Suh <[email protected]>
Description
Add capability to add user when someone interacts with a Slack bot without having logged in via the UI. This invovles:
has_web_logincolumn tousertable. This allows us to have logic where we allow a user to set their password with basic auth. Otherwise, a user would not know the placeholder password we set for them.How Has This Been Tested?
Accepted Risk
More database calls / Slack APIs in Slack bot + login flows.
Checklist: