-
Notifications
You must be signed in to change notification settings - Fork 7
use AuthUser.user_id as the browser_id #953
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import re | ||
| from typing import cast | ||
|
|
||
| from fastapi import FastAPI | ||
|
|
@@ -6,15 +7,16 @@ | |
| from mcp.server.auth.middleware.auth_context import AuthContextMiddleware | ||
| from mcp.server.auth.middleware.bearer_auth import BearerAuthBackend, RequireAuthMiddleware | ||
| from mcp.server.auth.provider import TokenVerifier | ||
| from pydantic import BaseModel, field_validator | ||
| from nanoid import generate | ||
| from pydantic import BaseModel, field_validator, model_validator | ||
| from starlette.datastructures import Headers | ||
| from starlette.middleware import Middleware | ||
| from starlette.middleware.authentication import AuthenticationMiddleware | ||
| from starlette.responses import RedirectResponse | ||
| from starlette.types import Receive, Scope, Send | ||
|
|
||
| from getgather.auth.provider import CustomOAuthProvider | ||
| from getgather.config import settings | ||
| from getgather.config import FRIENDLY_CHARS, settings | ||
|
|
||
|
|
||
| class RequireAuthMiddlewareCustom(RequireAuthMiddleware): | ||
|
|
@@ -99,14 +101,32 @@ class AuthUser(BaseModel): | |
| @field_validator("auth_provider") | ||
| @classmethod | ||
| def validate_auth_provider(cls, v: str) -> str: | ||
| if v not in [settings.FIRST_PARTY_OAUTH_PROVIDER_NAME, "google"]: | ||
| valid_providers = ( | ||
| [settings.FIRST_PARTY_OAUTH_PROVIDER_NAME, "google"] | ||
| if settings.auth_enabled | ||
| else [NO_AUTH_PROVIDER] | ||
| ) | ||
|
|
||
| if v not in valid_providers: | ||
| raise ValueError(f"Invalid auth provider: {v}") | ||
| return v | ||
|
|
||
| @model_validator(mode="after") | ||
| def validate_user_id(self) -> "AuthUser": | ||
| if len(self.user_id) > 54: | ||
| raise ValueError(f"User id is too long: {self.user_id}") | ||
| if not re.match(r"^[a-z0-9-]+$", self.user_id): | ||
| raise ValueError(f"User id contains invalid characters: {self.user_id}") | ||
| return self | ||
|
|
||
| @property | ||
| def user_id(self) -> str: | ||
| """Unique user name combining login and auth provider""" | ||
| return f"{self.sub}.{self.auth_provider}" | ||
| """ | ||
| Unique user name combining login and auth provider. | ||
| Only numbers, lowercase letters and dashes are allowed. | ||
| Maximum length is 54 characters. | ||
| """ | ||
| return f"{self.sub}-{self.auth_provider}" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why sub-auth provider rather than auth provider - sub? Just curious
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no strong preference, more like firstname - lastname |
||
|
|
||
| @classmethod | ||
| def from_user_id(cls, user_id: str) -> "AuthUser": | ||
|
|
@@ -121,8 +141,7 @@ def dump(self): | |
|
|
||
| def get_auth_user() -> AuthUser: | ||
| if not settings.auth_enabled: | ||
| # for testing only when auth is disabled | ||
| return AuthUser(sub="test_user", auth_provider=settings.FIRST_PARTY_OAUTH_PROVIDER_NAME) | ||
| return _get_user_for_no_auth() | ||
|
|
||
| token = get_access_token() | ||
| if not token: | ||
|
|
@@ -137,3 +156,12 @@ def get_auth_user() -> AuthUser: | |
| raise RuntimeError("Missing sub or provider in auth token") | ||
|
|
||
| return AuthUser(sub=sub, auth_provider=provider, name=name, email=email, app_name=app_name) | ||
|
|
||
|
|
||
| NO_AUTH_PROVIDER = "noauth" | ||
|
|
||
|
|
||
| def _get_user_for_no_auth() -> AuthUser: | ||
| """Fake auth user for when auth is disabled to keep the code consistent.""" | ||
| sub = generate(FRIENDLY_CHARS, 6) | ||
| return AuthUser(sub=sub, auth_provider=NO_AUTH_PROVIDER) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @faisalive @broerjuang Please confirm that this new format, e.g.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similar to gateway, demo apps will set the token header to where
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cool so it sounds like the user_id will always avoid the underscores which I think is needed |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you want to reverse parse it to check if the auth_provider is valid?