Conversation
Signed-off-by: Philip Miglinci <pmig@glasskube.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an admin-backed user management feature across the Go backend and Angular frontend, adding an is_admin role flag, admin-only API endpoints, and corresponding admin UI for managing users, emails, MFA, passwords, and archival status.
Changes:
- Add admin-only user management API endpoints (list/create/get/update/archive/unarchive, email management, MFA disable, password reset).
- Introduce
is_adminrole in user projections/events/JWT and expose it via/settings/user. - Add new Angular admin screens and refactor user settings into reusable “user-*” components backed by a shared port interface.
Reviewed changes
Copilot reviewed 86 out of 86 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/api/zz_generated.models.go | Adds generated API models for admin user management and isAdmin on UserResponse. |
| internal/users/repository.go | Wires new user events into projection updates. |
| internal/users/projections.go | Adds projection updates for is_admin changes and unarchive. |
| internal/users/mapping.go | Adds mapping for admin user list/detail DTOs. |
| internal/users/events.go | Adds UserAdminChanged and UserUnarchived events + includes is_admin in registration event. |
| internal/users/auth.go | Updates token generation to include is_admin; changes registration aggregate signature usage. |
| internal/users/aggregate.go | Adds IsAdmin state + commands/events for admin changes and unarchive. |
| internal/users/admin.go | Introduces UserService admin operations (CRUD-like + MFA/email/password/archive actions). |
| internal/server/zz_generated.server.go | Regenerates server interfaces/wrappers and routes for new admin endpoints. |
| internal/server/server.go | Stores DB pool on server to support admin authorization checks. |
| internal/server/endpoints_settings.go | Includes isAdmin in settings user responses. |
| internal/server/endpoints_admin_users.go | Implements admin user management endpoints with RequireAdmin checks. |
| internal/db/zz_generated.models.go | Adds IsAdmin field to the user projection model. |
| internal/db/zz_generated_schema.sql | Updates exported schema to include is_admin on user projection table. |
| internal/db/user_account.sql.go | Regenerates sqlc queries/models; adds list users query and is_admin updates. |
| internal/db/user_account.sql | Adds SQL for listing users and updating is_admin; adjusts archive update signature. |
| internal/db/migrations/sql/0_initial_schema.up.sql | Adds is_admin column to initial schema migration. |
| internal/dateierrors/errors.go | Adds ErrForbidden sentinel error. |
| internal/authn/middleware.go | Extracts is_admin claim into AuthInfo. |
| internal/authn/admin.go | Adds DB-backed RequireAdmin authorization helper. |
| internal/authjwt/jwt.go | Adds is_admin JWT claim and updates default token signature. |
| frontend/src/app/users/user-profile.component.ts | New reusable profile (name) editor component. |
| frontend/src/app/users/user-profile.component.html | Template for profile editor. |
| frontend/src/app/users/user-password.component.ts | New reusable password change/reset component. |
| frontend/src/app/users/user-password.component.html | Template for password component. |
| frontend/src/app/users/user-mfa.component.ts | New reusable self-service MFA setup/enable/disable component. |
| frontend/src/app/users/user-mfa.component.html | Template for MFA component. |
| frontend/src/app/users/user-emails.component.ts | New reusable email management component. |
| frontend/src/app/users/user-emails.component.html | Template for emails component. |
| frontend/src/app/users/user-data.port.ts | Introduces shared “port” abstraction for self/admin user operations. |
| frontend/src/app/users/initials.ts | Adds helper for rendering name initials. |
| frontend/src/app/settings/user-settings.component.ts | Refactors settings page to use new reusable user components. |
| frontend/src/app/settings/user-settings.component.html | Refactors settings template to compose new user components. |
| frontend/src/app/services/auth.service.ts | Adds is_admin claim typing and isAdmin computed. |
| frontend/src/app/services/admin-users.service.ts | Adds Angular service wrapper for admin user APIs. |
| frontend/src/app/nav/nav.component.ts | Exposes isAdmin for conditional admin navigation entry. |
| frontend/src/app/nav/nav.component.html | Shows “Admin” menu entry when isAdmin() is true. |
| frontend/src/app/guards/admin.guard.ts | Adds route guard for /admin/* routes. |
| frontend/src/app/app-logged-in.routes.ts | Adds /admin/users and /admin/users/:id routes guarded by adminGuard. |
| frontend/src/app/admin/admin-users-list.component.ts | Adds admin users list screen with filtering + create dialog. |
| frontend/src/app/admin/admin-users-list.component.html | Template for users list screen. |
| frontend/src/app/admin/admin-user-detail.component.ts | Adds admin user detail screen composing reusable user components + admin actions. |
| frontend/src/app/admin/admin-user-detail.component.html | Template for user detail screen. |
| frontend/src/app/admin/admin-role.component.ts | Adds admin role toggle component. |
| frontend/src/app/admin/admin-role.component.html | Template for role toggle. |
| frontend/src/app/admin/admin-mfa.component.ts | Adds admin-only MFA disable component. |
| frontend/src/app/admin/admin-mfa.component.html | Template for admin MFA component. |
| frontend/src/app/admin/admin-create-user-dialog.component.ts | Adds dialog for creating a user (incl. initial password/admin flag). |
| frontend/src/app/admin/admin-create-user-dialog.component.html | Template for create-user dialog. |
| frontend/src/app/admin/admin-archive.component.ts | Adds archive/unarchive component with undo flow. |
| frontend/src/app/admin/admin-archive.component.html | Template for archive/unarchive component. |
| frontend/src/api/models/user-response.ts | Regenerates TS model: adds isAdmin to UserResponse. |
| frontend/src/api/models/list-admin-users-response.ts | Adds generated TS model for admin user list response. |
| frontend/src/api/models/admin-user-list-item.ts | Adds generated TS model for admin user list item. |
| frontend/src/api/models/admin-update-user-request.ts | Adds generated TS model for admin update request. |
| frontend/src/api/models/admin-reset-password-request.ts | Adds generated TS model for admin reset password request. |
| frontend/src/api/models/admin-create-user-request.ts | Adds generated TS model for admin create user request. |
| frontend/src/api/models.ts | Exports newly generated admin models. |
| frontend/src/api/functions.ts | Exports newly generated admin operation functions. |
| frontend/src/api/fn/operations/update-user-admin.ts | Generated client for admin update user. |
| frontend/src/api/fn/operations/unarchive-user-admin.ts | Generated client for admin unarchive user. |
| frontend/src/api/fn/operations/set-primary-user-email-admin.ts | Generated client for admin set-primary-email. |
| frontend/src/api/fn/operations/reset-user-password-admin.ts | Generated client for admin reset password. |
| frontend/src/api/fn/operations/remove-user-email-admin.ts | Generated client for admin remove email. |
| frontend/src/api/fn/operations/list-users-admin.ts | Generated client for admin list users. |
| frontend/src/api/fn/operations/list-user-emails-admin.ts | Generated client for admin list user emails. |
| frontend/src/api/fn/operations/get-user-admin.ts | Generated client for admin get user. |
| frontend/src/api/fn/operations/disable-user-mfa-admin.ts | Generated client for admin disable MFA. |
| frontend/src/api/fn/operations/create-user-admin.ts | Generated client for admin create user. |
| frontend/src/api/fn/operations/archive-user-admin.ts | Generated client for admin archive user. |
| frontend/src/api/fn/operations/add-user-email-admin.ts | Generated client for admin add email. |
| api/paths/admin_users.yaml | Adds OpenAPI paths for admin list/create users. |
| api/paths/admin_users_id.yaml | Adds OpenAPI paths for admin get/update user. |
| api/paths/admin_users_id_password.yaml | Adds OpenAPI path for admin password reset. |
| api/paths/admin_users_id_mfa_disable.yaml | Adds OpenAPI path for admin MFA disable. |
| api/paths/admin_users_id_emails.yaml | Adds OpenAPI paths for admin list/add emails. |
| api/paths/admin_users_id_emails_emailid.yaml | Adds OpenAPI path for admin remove email. |
| api/paths/admin_users_id_emails_emailid_primary.yaml | Adds OpenAPI path for admin set primary email. |
| api/paths/admin_users_id_archive.yaml | Adds OpenAPI paths for admin archive/unarchive user. |
| api/openapi.yaml | Registers admin paths in the OpenAPI root document. |
| api/components/schemas/UserResponse.yaml | Adds required isAdmin field to user response schema. |
| api/components/schemas/ListAdminUsersResponse.yaml | Adds schema for admin list users response. |
| api/components/schemas/AdminUserListItem.yaml | Adds schema for admin user list/detail DTO. |
| api/components/schemas/AdminUpdateUserRequest.yaml | Adds schema for admin update request. |
| api/components/schemas/AdminResetPasswordRequest.yaml | Adds schema for admin reset password request. |
| api/components/schemas/AdminCreateUserRequest.yaml | Adds schema for admin create request. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+151
to
+154
| if err := agg.Register( | ||
| userID, input.Name, input.Email, emailID, passwordHash, passwordSalt, | ||
| true, time.Now(), | ||
| ); err != nil { |
Contributor
Author
There was a problem hiding this comment.
this is intentional for now, else the first user will not be an admin
Comment on lines
9
to
13
| name TEXT NOT NULL, | ||
| password_hash BYTEA NOT NULL, | ||
| password_salt BYTEA NOT NULL, | ||
| is_admin BOOLEAN NOT NULL DEFAULT true, | ||
| mfa_secret TEXT, |
Comment on lines
+255
to
+261
| if err := s.userService.AdminDisableMFA(ctx, request.Id); err != nil { | ||
| if errors.Is(err, dateierrors.ErrMFANotEnabled) { | ||
| return DisableUserMFAAdmin403Response{}, nil | ||
| } | ||
| slog.Error("admin disable mfa error", "error", err) | ||
| return nil, err | ||
| } |
Comment on lines
+5
to
+9
| export const adminGuard: CanActivateFn = () => { | ||
| const auth = inject(AuthService); | ||
| const router = inject(Router); | ||
| if (auth.isAdmin()) return true; | ||
| return router.createUrlTree(['/']); |
Contributor
Author
There was a problem hiding this comment.
we will improve this later on
Signed-off-by: Philip Miglinci <pmig@glasskube.com>
Signed-off-by: Philip Miglinci <pmig@glasskube.com>
kosmoz
reviewed
May 7, 2026
Comment on lines
+56
to
+57
| /api/v1/admin/users/{id}/emails/{emailId}/primary: | ||
| $ref: ./paths/admin_users_id_emails_emailid_primary.yaml |
Contributor
There was a problem hiding this comment.
I think this is not good API design. the primary email is a property of the user not the email, so the endpoint should be something like
PATCH /users/{id}
{"primaryEmailId": "..."}
Contributor
There was a problem hiding this comment.
Maybe I don't understand what an archived user is, but assuming it means to enable/disable login for a user:
- I think enable/disable would be a better word than archive/unarchive
- The API should be something like
PATCH /users/{id}
{"enabled": true}
Contributor
There was a problem hiding this comment.
same as in the other PR: no error logging on individual endpoints please
| // has is_admin=true in the projection. The flag is verified against the | ||
| // database (not the JWT) so demotion takes effect on the next call. | ||
| // Returns dateierrors.ErrForbidden if the caller is not an admin. | ||
| func RequireAdmin(ctx context.Context, q *db.Queries) (AuthInfo, error) { |
Contributor
There was a problem hiding this comment.
While it absolutely makes sense to check the database for privileges, I don't think this is the right way to do it.
I think long-term we would want to fetch the user from the DB for every request anyways so this extra round trip is unnecessary. For now I would suggest to just use AuthInfo from the context instead. If you really want to check the DB, do it properly and fetch the user on every request.
With this change, the server doesn't need a pgxpool directly anymore, which is something we should definitely avoid to prevent stronger coupling
|
|
||
| func (s *UserService) GetUserForAdmin( | ||
| ctx context.Context, userID uuid.UUID, | ||
| ) (db.ListUserAccountProjectionsRow, error) { |
Contributor
There was a problem hiding this comment.
I know the user service already does it like this, but for datei we have the service perform type mapping and return API types instead of db types. I think this should be the preferable approach, as the API types are more under our control via the OpenAPI definitions.
This PR is probably not the right place to refactor this (it is already big enough :^)) but I think we should refactor the user service to also return API types instead of db types.
| <mat-card | ||
| appearance="outlined" | ||
| matRipple | ||
| class="cursor-pointer relative transition-colors hover:mat-bg-surface-container" |
Contributor
There was a problem hiding this comment.
This doesn't work because it combines tailwind and angular material
hover:mat-bg-surface-container
| class="block focus-visible:outline focus-visible:outline-offset-2 mat-corner-md no-underline" | ||
| style="outline-color: var(--mat-sys-primary)" | ||
| > | ||
| <mat-card |
Contributor
There was a problem hiding this comment.
I think this looks kind of bad tbh… the subtitle text is way too heavy and the spacing is all wrong. I think the card styles might not be what we want here. The material page has some better examples under lists (https://m3.material.io/components/lists/overview) but list items have no container. maybe we can combine the card and list styles of angular material to get the best of both worlds?
Comment on lines
+59
to
+69
| this.loading.set(true); | ||
| this.admin.listUsers().subscribe({ | ||
| next: (users) => { | ||
| this.users.set(users); | ||
| this.loading.set(false); | ||
| }, | ||
| error: () => { | ||
| this.loading.set(false); | ||
| this.error.set(true); | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
should use resource/rxResource I think
|
|
||
| <app-user-profile [port]="port()" [name]="u.name" (changed)="load()" /> | ||
| <app-user-emails [port]="port()" /> | ||
| @if (!isSelf()) { |
Contributor
There was a problem hiding this comment.
maybe show an info text when editing self
| load() { | ||
| const id = this.userId(); | ||
| if (!id) { | ||
| this.loading.set(false); |
…agement Signed-off-by: Philip Miglinci <pmig@glasskube.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.