-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(demo): adds middleware to log user in automatically #24317
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
Conversation
markstory
left a comment
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.
Would be good to have some tests around member selection and fail safes.
src/sentry/demo/middleware.py
Outdated
| # on organization pages | ||
| def process_view(self, request, view_func, view_args, view_kwargs): | ||
| if not settings.DEMO_MODE: | ||
| return |
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.
I would raise an exception here so that the application fails to start as an additional precaution.
| return | ||
|
|
||
| # if authed, no action required | ||
| if request.user.is_authenticated() and request.user.is_active: |
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.
If the user is logged in but not part of the demo org would they hit errors viewing the demo org data? You might want to coerce their session into the demo org they are looking at.
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.
@markstory good point. It's sort of an edge case but it's possible someone could be logged in for on demo but receive a link for another org's demo. In that case, we should switch the user.
This PR adds functionality to log a user into the demo automatically if they land on an organization page in the demo. The purpose of this is to allow users to share links with other people when trying out the demo. While this would be a security vulnerability in SaaS, this should be totally fine in the demo instance for a few reasons. First, one would have to guess an org slug to get access which wouldn't be easy because we auto-generate them using
petname. Secondly, we only givememberaccess meaning that a user who guesses a slug would have limited abilities to take destructive action on someone else's demo.