-
Notifications
You must be signed in to change notification settings - Fork 38
feat(controlplane): organization invite system #404
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
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
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.
Note for reviewers. I left some comments in the PR as guidance of what's worth reviewing and not autogenerated.
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
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.
Please make sure we validate deleted_at before creating a membership. Apart from that LGTM!
|
||
func newOrganizationInvitationSentCmd() *cobra.Command { | ||
cmd := &cobra.Command{ | ||
Use: "sent", |
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 not just list
like we do in other places?
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.
The reason I didn't want to put list is so it doesn't get confused with listing all invitations for an org
, since this command returns only the invites,you as an user sent.
In any case, I've renamed it to list
and made it clear in the description of the command what it does.
Thanks!
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 was about to post a similar comment, even though not quite the same.
If we use sent
, maybe instead of "creating" and invite, we "send" it
|
||
// user is not a member of the org, create the membership | ||
if !alreadyMember { | ||
uc.logger.Infow("msg", "Adding member", "invite_id", invite.ID.String(), "org_id", invite.Org.ID, "user_id", user.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.
are we checking deleted_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.
The invites queries skip automatically the deleted entries..
So invites, err := uc.repo.PendingInvites(ctx, receiverEmail)
will skip those
Is that the deletedAt
you were talking about, or any other soft-deleted entity?
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
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.
🚀
First cut of an invite system where a user can invite others to join a given organization. Closes #401
NOTE: The auto-acceptance mechanism is just a first pass to simplify the implementation. In the future, we'll give control to the receiver to accept or decline offers explicitly #401. For now, we think this is okay because users are usually invited withing their own Chainloop instance
CLI Changes
Org Invite commands
$ chainloop org invite revoke --id 880da13c-3ae1-4ec0-9e87-0ae4eafe8e09 INF Invite Revoked!
biz layer
Creation logic
The handler implementation will a) run static validations, b) check if the user has permission to invite to the given org, c) check that an invitation doesn't exist already, and 4) create an entry in the DB in a pending state.
Revocation logic, in fact, will soft-delete the item by setting a
deleted_at
timestamp.Auto-acceptance logic
An user, on login will get automatically added to the organizations they have been invited to. These will appear in the
org list
commandWhich then can be chosen using
chainloop org set --id ...
data layer
It adds an
org_invites
persistence table.