mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(coderd): allow user-admin password resets to succeed (#26537)
User Admin password resets could update the target user's hashed password but fail while revoking that user's API keys. The transaction then rolled back and returned HTTP 500, so the password was never changed. Add a user-scoped API key revoker actor and use it in both password reset flows so key revocation succeeds without broader system auth. Refs: https://linear.app/codercom/issue/PLAT-316
This commit is contained in:
@@ -453,6 +453,26 @@ var (
|
||||
}.WithCachedASTValue()
|
||||
}
|
||||
|
||||
subjectAPIKeyRevoker = func(userID uuid.UUID) rbac.Subject {
|
||||
return rbac.Subject{
|
||||
Type: rbac.SubjectTypeAPIKeyRevoker,
|
||||
FriendlyName: "API Key Revoker",
|
||||
ID: userID.String(),
|
||||
Roles: rbac.Roles([]rbac.Role{
|
||||
{
|
||||
Identifier: rbac.RoleIdentifier{Name: "apikeyrevoker"},
|
||||
DisplayName: "API Key Revoker",
|
||||
Site: []rbac.Permission{},
|
||||
User: rbac.Permissions(map[string][]policy.Action{
|
||||
rbac.ResourceApiKey.Type: {policy.ActionDelete},
|
||||
}),
|
||||
ByOrgID: map[string]rbac.OrgPermissions{},
|
||||
},
|
||||
}),
|
||||
Scope: rbac.ScopeAll,
|
||||
}.WithCachedASTValue()
|
||||
}
|
||||
|
||||
subjectSystemRestricted = rbac.Subject{
|
||||
Type: rbac.SubjectTypeSystemRestricted,
|
||||
FriendlyName: "System",
|
||||
@@ -845,6 +865,12 @@ func AsSubAgentAPI(ctx context.Context, orgID uuid.UUID, userID uuid.UUID) conte
|
||||
return As(ctx, subjectSubAgentAPI(userID, orgID))
|
||||
}
|
||||
|
||||
// AsAPIKeyRevoker returns a context with an actor that can revoke API
|
||||
// keys owned by the specified user, and nothing else.
|
||||
func AsAPIKeyRevoker(ctx context.Context, userID uuid.UUID) context.Context {
|
||||
return As(ctx, subjectAPIKeyRevoker(userID))
|
||||
}
|
||||
|
||||
// AsSystemRestricted returns a context with an actor that has permissions
|
||||
// required for various system operations (login, logout, metrics cache).
|
||||
// DO NOT USE THIS UNLESS YOU HAVE ABSOLUTELY NO OTHER CHOICE. Prefer using a
|
||||
|
||||
@@ -7259,6 +7259,39 @@ func TestAuthorizeProvisionerJob_SystemFastPath(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestAsAPIKeyRevoker(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
userID := uuid.New()
|
||||
otherUserID := uuid.New()
|
||||
ctx := dbauthz.AsAPIKeyRevoker(context.Background(), userID)
|
||||
actor, ok := dbauthz.ActorFromContext(ctx)
|
||||
require.True(t, ok, "actor must be present")
|
||||
|
||||
auth := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry())
|
||||
|
||||
t.Run("OwnedAPIKeys", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
resource := rbac.ResourceApiKey.WithOwner(userID.String())
|
||||
for _, action := range rbac.ResourceApiKey.AvailableActions() {
|
||||
err := auth.Authorize(ctx, actor, action, resource)
|
||||
if action == policy.ActionDelete {
|
||||
require.NoError(t, err, "owned api keys should allow %s", action)
|
||||
continue
|
||||
}
|
||||
require.Error(t, err, "owned api keys should deny %s", action)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("OtherUsersAPIKeys", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
err := auth.Authorize(ctx, actor, policy.ActionDelete, rbac.ResourceApiKey.WithOwner(otherUserID.String()))
|
||||
require.Error(t, err, "other users' api keys should not be deletable")
|
||||
})
|
||||
}
|
||||
|
||||
func TestAsChatd(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user