-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add docs for user management and invitations #3837
Add docs for user management and invitations #3837
Conversation
Note: I ended up running |
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.
Thanks @evankanderson , just a small nit, feel free to ignore
Co-authored-by: Puerco <puerco@users.noreply.github.com>
change users roles within the project. | ||
|
||
When creating an account, Minder will automatically create a default project for | ||
you, unless you are accepting an invitation to an existing project. You can |
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.
Unfortunately this is not like that right now. We still create a project, we just don't go through the onboarding flow and also show by default the oldest project in the UI (which is the one the user accepted the invite for).
We had a discussion with @jhrozek that we should separate the project creation part from the user creation part, ref. #3826 (implying changes on the client's side - UI and CLI)
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.
@ethomson -- are we okay shipping invitations with this change from the PRD & tech design, or do we need to address it before release?
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.
Another note - if the user accepted an invite, but then selects their default project from the drop down menu they will be taken through the onboarding flow for it since it's what the front end does for every new project.
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 think it's really surprising that we new up a secret project behind the scenes that nobody asked for.
- We have a metric around project creation and how many have rules and profiles associated with them (versus not), and this will annihilate that metric. 🙃
- The UI should show the most recent project, not the oldest project. (Again, the FE needs a data store for its own business logic, and doesn't have one.)
Can you give me a rough cost on separating user creation from project creation?
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 think we need to figure out what the API call is that creates the user without the project. I need to look and see if it makes sense for ResolveInvitation
to potentially set up a new user if it's called by someone who is known to Keycloak but not to Minder. If so, the flow for getting a Minder account would be either:
ResolveInvitation -> ...
OR
CreateUser -> ...
Right now we're measuring projects primarily, so we probably wouldn't really "see" the additional users on a project from a metrics PoV, which may be okay in general.
The interesting question is what middleware and other plumbing might break by making ResolveInvitation not require a User object already stored in the database.
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 think this will be a small change, if I'm right, I'm about 2/3rds done now (plus tests, so really 40% done).
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 made it a medium-size change, and it will need a client release: #3909
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.
Also @kantord for a heads-up; I commented on #3909, but the frontend should not call CreateUser
before calling ResolveInvitation
-- ResolveInvitation
will take care of any bookkeeping needed in terms of getting user information. Note that before calling ResolveInvitation
, the GetUser
call will also fail.
…tion where I missed it. Co-authored-by: Radoslav Dimitrov <radoslav@stacklok.com>
d261f76
into
mindersec:3795-add-docs-for-user-management-and-invitations
* Add conceptual docs on projects, document how to create new projects, because I was there * Add role requirements to how-to prereqs * Clarify one role per project * Rewrite adding users to describe invitations * Undo excessive prettier formatting * Update docs/docs/how-to/create_project.md Co-authored-by: Puerco <puerco@users.noreply.github.com> * Switch naming from Stacklok (vendor) to Minder (project) in documentation where I missed it. Co-authored-by: Radoslav Dimitrov <radoslav@stacklok.com> --------- Co-authored-by: Radoslav Dimitrov <radoslav@stacklok.com> Co-authored-by: Puerco <puerco@users.noreply.github.com>
* Add conceptual docs on projects, document how to create new projects, because I was there * Add role requirements to how-to prereqs * Clarify one role per project * Rewrite adding users to describe invitations * Undo excessive prettier formatting * Update docs/docs/how-to/create_project.md * Switch naming from Stacklok (vendor) to Minder (project) in documentation where I missed it. --------- Co-authored-by: Evan Anderson <evan@stacklok.com> Co-authored-by: Puerco <puerco@users.noreply.github.com>
Summary
Document the new user invitations feature
Fixes #3795
Change Type
Mark the type of change your PR introduces:
Testing
N/A
Review Checklist: