-
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?
Conversation
| 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) |
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.
@faisalive @broerjuang Please confirm that this new format, e.g. xyz123-noauth will not break signin etc, for demo apps.
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.
similar to gateway, demo apps will set the token header to ario_APPKEY_xyz123
then the user_id will be like xyz123-ario
where ario is the first party auth provider name (used to getgather), APPKEY is pre-defined random key, xyz123 is generated by express session id
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.
cool so it sounds like the user_id will always avoid the underscores which I think is needed
| Only numbers, lowercase letters and dashes are allowed. | ||
| Maximum length is 54 characters. | ||
| """ | ||
| return f"{self.sub}-{self.auth_provider}" |
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.
why sub-auth provider rather than auth provider - sub? Just curious
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.
no strong preference, more like firstname - lastname
| return v | ||
|
|
||
| @model_validator(mode="after") | ||
| def validate_user_id(self) -> "AuthUser": |
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?
makes it easy to find an AuthUser's chrome container/app, the names will be like
chrome-123456-googleif auth is disabled, use a fake user with random user_id, the container name will be like
chrome-xyz123-noauthverified with tests/manual.py