Skip to content

Add profile view, edit implementation #1137

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.

Sign up for GitHub

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

Open
lashinijay wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add profile view, edit implementation #1137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0632873
Add profile view, edit implementation
lashinijay Jun 22, 2026
23d2ab6
fix build issues
lashinijay Jun 22, 2026
7bb0f2b
Merge branch 'main' into profile-view
lashinijay Jun 22, 2026
0067225
fix build issues
lashinijay Jun 22, 2026
122925b
Merge branch 'main' into profile-view
lashinijay Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions agent-manager-service/api/identity_routes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func registerIdentityRoutes(rr *middleware.RouteRegistrar, ctrl controllers.Iden
rr.HandleFuncWithValidationAndAnyAuthz("GET /orgs/{orgName}/identities/users", ctrl.ListUsers, rbac.OrgInviteMember, rbac.OrgRemoveMember)
rr.HandleFuncWithValidationAndAuthz("POST /orgs/{orgName}/identities/users/invite", rbac.OrgInviteMember, ctrl.InviteUser)
rr.HandleFuncWithValidationAndAuthz("POST /orgs/{orgName}/identities/users", rbac.OrgInviteMember, ctrl.CreateUser)
rr.HandleFuncWithValidationAndAuthz("PUT /orgs/{orgName}/identities/users/{userID}/profile", rbac.ProfileUpdateAttributes, ctrl.UpdateCurrentUserProfile)
rr.HandleFuncWithValidationAndAnyAuthz("GET /orgs/{orgName}/identities/users/{userID}", ctrl.GetUser, rbac.OrgInviteMember, rbac.OrgRemoveMember)
rr.HandleFuncWithValidationAndAuthz("PUT /orgs/{orgName}/identities/users/{userID}", rbac.OrgInviteMember, ctrl.UpdateUser)
rr.HandleFuncWithValidationAndAuthz("DELETE /orgs/{orgName}/identities/users/{userID}", rbac.OrgRemoveMember, ctrl.DeleteUser)
Expand Down
34 changes: 34 additions & 0 deletions agent-manager-service/controllers/identity_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type IdentityController interface {
GetUserGroups(w http.ResponseWriter, r *http.Request)
GetUserRoles(w http.ResponseWriter, r *http.Request)
InviteUser(w http.ResponseWriter, r *http.Request)
UpdateCurrentUserProfile(w http.ResponseWriter, r *http.Request)

// Groups
ListGroups(w http.ResponseWriter, r *http.Request)
Expand Down Expand Up @@ -1298,6 +1299,39 @@ func (c *identityController) ListAMPPermissions(w http.ResponseWriter, r *http.R
utils.WriteSuccessResponse(w, http.StatusOK, map[string]any{"permissions": perms, "resourceServerId": rsID})
}

// UpdateCurrentUserProfile updates the current user's profile information
func (c *identityController) UpdateCurrentUserProfile(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
log := logger.GetLogger(ctx)

userID := r.PathValue(utils.PathParamUserID)
if userID == "" {
utils.WriteErrorResponse(w, http.StatusBadRequest, "User ID is required")
return
}

var body spec.UpdateUserRequest
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
utils.WriteErrorResponse(w, http.StatusBadRequest, "Invalid request body")
return
}

updatedUser, err := c.client.UpdateUser(ctx, userID, thundersvc.UpdateUserRequest{
Attributes: *body.Attributes,
})
Comment on lines +1307 to +1321

@coderabbitai coderabbitai Bot Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Enforce self-only profile updates before calling the admin client.

This handler trusts the path userID and updates that account directly, so any caller with profile:update-attributes can target other users by ID. For a “current user profile” endpoint, bind the target user to the authenticated subject (or use /me) and reject mismatches.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agent-manager-service/controllers/identity_controller.go` around lines 1307 -
1321, The UpdateUser handler has an authorization vulnerability where any caller
with the profile:update-attributes permission can update any user's profile by
specifying a different userID in the path. Extract the authenticated user
subject from the request context and validate that it matches the userID path
parameter before proceeding with the UpdateUser call. If they do not match,
return an appropriate error response (such as Forbidden status) to ensure users
can only update their own profiles unless they have additional admin privileges.

Comment on lines +1319 to +1321

@coderabbitai coderabbitai Bot Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard against nil attributes before dereferencing.

body.Attributes is a pointer type in the request model; dereferencing it directly can panic on valid-but-empty/null payloads.

💡 Suggested fix 
 var body spec.UpdateUserRequest
 if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
 	utils.WriteErrorResponse(w, http.StatusBadRequest, "Invalid request body")
 	return
 }
+if body.Attributes == nil {
+	utils.WriteErrorResponse(w, http.StatusBadRequest, "Attributes are required")
+	return
+}

 updatedUser, err := c.client.UpdateUser(ctx, userID, thundersvc.UpdateUserRequest{
-	Attributes: *body.Attributes,
+	Attributes: body.GetAttributes(),
 })
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agent-manager-service/controllers/identity_controller.go` around lines 1319 -
1321, The UpdateUser call is dereferencing body.Attributes without checking if
it is nil first, which will cause a panic when the request contains a null or
empty attributes payload. Add a nil check before dereferencing body.Attributes
in the UpdateUser method call. If body.Attributes is nil, either pass an empty
struct, skip the update, or handle it according to your business logic
requirements.

if err != nil {
if thundersvc.IsNotFound(err) {
utils.WriteErrorResponse(w, http.StatusNotFound, "User not found")
return
}
log.Error("UpdateCurrentUserProfile failed", "userID", userID, "error", err)
utils.WriteErrorResponse(w, http.StatusInternalServerError, "Failed to update user profile")
return
}

utils.WriteSuccessResponse(w, http.StatusOK, updatedUser)
}

// validateUserOwnership checks if a user belongs to the caller's OU
func validateUserOwnership(w http.ResponseWriter, ctx context.Context, user *thundersvc.ThunderUser, callerOuID string) bool {
if user.OuID != "" && user.OuID != callerOuID {
Expand Down
57 changes: 57 additions & 0 deletions agent-manager-service/docs/api_v1_openapi.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7932,6 +7932,63 @@ paths:
schema:
$ref: "#/components/schemas/ErrorResponse"

/orgs/{orgName}/identities/users/{userId}/profile:
put:
summary: Update user profile and credentials
operationId: updateCurrentUserProfile
tags:
- Identity - Users
parameters:
- name: orgName
in: path
required: true
description: Organization name
schema:
type: string
- name: userId
in: path
required: true
description: User ID
schema:
type: string
Comment on lines +7935 to +7953

@coderabbitai coderabbitai Bot Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Resolve self-service contract ambiguity in updateCurrentUserProfile.

operationId says “current user”, but the path requires a caller-supplied {userId}. That invites cross-user update attempts unless the backend strictly enforces token subject == userId (or explicitly allows elevated-role overrides). Please make the contract unambiguous: either derive user identity from auth context (drop {userId}) or rename/document this as a general user-update endpoint with explicit authorization semantics.

Suggested OpenAPI direction (self-service variant) 
-  /orgs/{orgName}/identities/users/{userId}/profile:
+  /orgs/{orgName}/identities/users/me/profile:
     put:
       summary: Update user profile and credentials
       operationId: updateCurrentUserProfile
       tags:
         - Identity - Users
       parameters:
         - name: orgName
           in: path
           required: true
           description: Organization name
           schema:
             type: string
-        - name: userId
-          in: path
-          required: true
-          description: User ID
-          schema:
-            type: string
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/orgs/{orgName}/identities/users/{userId}/profile:
put:
summary: Update user profile and credentials
operationId: updateCurrentUserProfile
tags:
- Identity - Users
parameters:
- name: orgName
in: path
required: true
description: Organization name
schema:
type: string
- name: userId
in: path
required: true
description: User ID
schema:
type: string
/orgs/{orgName}/identities/users/me/profile:
put:
summary: Update user profile and credentials
operationId: updateCurrentUserProfile
tags:
- Identity - Users
parameters:
- name: orgName
in: path
required: true
description: Organization name
schema:
type: string
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agent-manager-service/docs/api_v1_openapi.yaml` around lines 7924 - 7942, The
operationId updateCurrentUserProfile suggests a self-service endpoint for
updating the authenticated user's own profile, but the path parameter {userId}
allows callers to specify which user to update, creating ambiguity about
authorization intent. Resolve this by either removing the {userId} path
parameter and deriving user identity from the authentication context (making it
a true self-service endpoint), or renaming the operationId to reflect a general
user-update operation and explicitly documenting the authorization semantics
(e.g., whether role-based access allows elevated users to modify other users'
profiles).

requestBody:
required: true
content:
application/json:
schema:
$ref: "#/components/schemas/UpdateUserRequest"
responses:
"200":
description: User profile and credentials updated successfully
content:
application/json:
schema:
$ref: "#/components/schemas/UserResponse"
"400":
description: Invalid request
content:
application/json:
schema:
$ref: "#/components/schemas/ErrorResponse"
"403":
description: Forbidden - missing org context
content:
Comment on lines +7973 to +7975

@coderabbitai coderabbitai Bot Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align the 403 response description with adjacent user endpoints.

Line 7963 currently documents only “missing org context”, while sibling user endpoints also include “user does not belong to organization.” Keeping this consistent avoids client-side error-handling drift for the same identity domain.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agent-manager-service/docs/api_v1_openapi.yaml` around lines 7962 - 7964, The
403 response description is incomplete and inconsistent with adjacent user
endpoints. Update the description value in the 403 response section to include
both "missing org context" and "user does not belong to organization" to match
the pattern used in sibling user endpoint definitions elsewhere in the OpenAPI
specification, ensuring consistent error documentation across the identity
domain.

application/json:
schema:
$ref: "#/components/schemas/ErrorResponse"
"404":
description: User not found
content:
application/json:
schema:
$ref: "#/components/schemas/ErrorResponse"
"500":
description: Internal server error
content:
application/json:
schema:
$ref: "#/components/schemas/ErrorResponse"

/orgs/{orgName}/identities/groups:
get:
summary: List groups in an organization
Expand Down
5 changes: 5 additions & 0 deletions agent-manager-service/rbac/permissions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,8 @@ const (
CatalogRead Permission = "catalog:read"
RepositoryRead Permission = "repository:read"
)

// Profile permissions
const (
ProfileUpdateAttributes Permission = "profile:update-attributes"
)
4 changes: 4 additions & 0 deletions agent-manager-service/rbac/predefined_roles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ var PredefinedRolePermissions = map[string][]Permission{
RoleCreate, RoleRead, RoleUpdate, RoleDelete,
GroupCreate, GroupRead, GroupUpdate, GroupDelete,
CatalogRead, RepositoryRead,
ProfileUpdateAttributes,
},

RoleDeveloper: {
Expand All @@ -73,6 +74,7 @@ var PredefinedRolePermissions = map[string][]Permission{
MonitorScoreRead,
ObservabilityProjectDashboard, ObservabilityInfraMetric,
CatalogRead, RepositoryRead,
ProfileUpdateAttributes,
},

RoleAILead: {
Expand All @@ -91,6 +93,7 @@ var PredefinedRolePermissions = map[string][]Permission{
MonitorRead, MonitorScoreRead,
ObservabilityOrgDashboard, ObservabilityProjectDashboard, ObservabilityGuardrailMetric,
CatalogRead,
ProfileUpdateAttributes,
},

RolePlatformEngineer: {
Expand All @@ -107,5 +110,6 @@ var PredefinedRolePermissions = map[string][]Permission{
MonitorRead, MonitorScoreRead,
ObservabilityOrgDashboard, ObservabilityProjectDashboard, ObservabilityInfraMetric,
CatalogRead,
ProfileUpdateAttributes,
},
}
160 changes: 160 additions & 0 deletions agent-manager-service/spec/api_identity_users.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading