-
Notifications
You must be signed in to change notification settings - Fork 38
feat(users): Store user access on user's table #1923
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: Javier Rodriguez <javier@chainloop.dev>
Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
u.logger.Infow("msg", "Syncing user access") | ||
|
||
// Count the number of users with restricted access | ||
usersWithRestrictedAccess, err := u.userRepo.CountUsersWithRestrictedAccess(ctx) |
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.
move inside
// updateUserAccessBasedOnAllowList updates the access restriction status of all users based on the allowlist | ||
func (u *UserAccessSyncerUseCase) updateUserAccessBasedOnAllowList(ctx context.Context, usersWithRestrictedAccess int) error { | ||
// If the allowlist is empty and there are users with restricted access, give access to those users | ||
if u.allowList != nil && len(u.allowList.GetRules()) == 0 && usersWithRestrictedAccess > 0 { |
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.
we are missing the default configuration. if allowlist not set but nothing to update
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.
and I'd remove this case since it only applies once
|
||
// updateUserAccessRestriction updates the access restriction status of a user | ||
func (u *UserAccessSyncerUseCase) updateUserAccessRestriction(ctx context.Context, user *User, isAllowListDeactivated bool) error { | ||
allow, err := UserEmailInAllowlist(u.allowList.GetRules(), user.Email) |
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.
we do not need to do it if deactivated
Annotations(&entsql.Annotation{ | ||
Default: "CURRENT_TIMESTAMP", | ||
}), | ||
field.Bool("has_restricted_access").Default(true).Comment("Whether the user is blocked from accessing the system"), |
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 denormalized access state based on allow_list_settings
func (User) Indexes() []ent.Index { | ||
return []ent.Index{ | ||
index.Fields("has_restricted_access").Annotations( | ||
entsql.IndexWhere("has_restricted_access IS true"), |
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.
remove optimization
Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
Updated the PR to:
|
Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
This patch adds a new column,
has_restricted_access
, to theUsers
table. While not currently in use, it is synchronized with the allowlist configured on the server.A background goroutine runs at startup, syncing the
Users
table with the allowlist every 10 seconds.As we have in the allowlist middleware, if the allowlist is empty, we allow all users to get in. In this scenario, it means to update all users
has_restricted_access=true
to switch itfalse
.