Removed: user_management feature flag #5970

Mujib-Ahasan · 2025-11-13T17:39:15Z

Summary

The default value of user_management flag is false now. But related functionalities sets it to true. This PR is raised for the release of the flag.
Fixes #5436

Signed-off-by: Mujib Ahasan <[email protected]>

Mujib-Ahasan · 2025-11-13T17:43:35Z

Hey @evankanderson, this is the very first push to fix the issue, Please confirm if code changes are correct or not. Although few test cases are not passing in https://github.com/mindersec/minder/blob/main/internal/controlplane/handlers_authz_test.go, could you please guide what changes I need to make here also??

evankanderson

I've approved the automated test runs, but you probably want to remove the dead code at least (that will probably trigger lint failures). If there are other test failures, I'll take a look after my flight tonight (or maybe sooner).

evankanderson · 2025-11-13T19:56:59Z

internal/controlplane/handlers_authz.go

+			// Leaving the role assignment empty as it's an invitation
+			Invitation: invitation,
+		}, nil
 		return nil, util.UserVisibleError(codes.Unimplemented, "user management is not enabled")


This code is now unreachable, and should be removed.

Thank you for pointing out.

evankanderson · 2025-11-13T20:01:34Z

internal/controlplane/handlers_authz.go

+			Invitation: deletedInvitation,
+		}, nil
+
 		return nil, util.UserVisibleError(codes.Unimplemented, "user management is not enabled")


This code is also dead and should be removed.

evankanderson · 2025-11-13T20:02:14Z

internal/controlplane/handlers_authz.go

+			},
+		}, nil
+
 		return nil, util.UserVisibleError(codes.Unimplemented, "user management is not enabled")


More dead code

Signed-off-by: Mujib Ahasan <[email protected]>

evankanderson · 2025-11-19T20:40:05Z

This appears to have failing tests, possibly because enabling the user_management flag causes direct adds of users to projects without their consent to fail. You may simply need to convert these cases to expected errors. (Depending on resulting coverage, we might decide that we need to update some tests to invite users rather than direct-adding them.)

📦 github.com/mindersec/minder/internal/controlplane
coverage: 10.2% of statements in ./internal/..., ./pkg/...
❌ TestAssignRole (0s)
❌ TestAssignRole/request_with_subject_creates_role_assignment (0s)
      handlers_authz_test.go:894: 
          	Error Trace:	/home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:894
          	Error:      	Received unexpected error:
          	            	Code: 12
          	            	Name: UNIMPLEMENTED
          	            	Description: Unimplemented
          	            	Details: human users may only be added by invitation
          	Test:       	TestAssignRole/request_with_subject_creates_role_assignment
      controller.go:251: missing call(s) to *mock_roles.MockRoleService.CreateRoleAssignment(is anything, is anything, is anything, is anything, is equal to {user    } (auth.Identity), is equal to admin (authz.Role)) /home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:839
      controller.go:251: aborting test due to missing call(s)
❌ TestRoleManagement (0s)
❌ TestRoleManagement/IDP_resolution (1.07s)
      handlers_authz_test.go:621: 
          	Error Trace:	/home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:621
          	Error:      	Received unexpected error:
          	            	Code: 12
          	            	Name: UNIMPLEMENTED
          	            	Description: Unimplemented
          	            	Details: human users may only be added by invitation
          	Test:       	TestRoleManagement/IDP_resolution
      handlers_authz_test.go:621: 
          	Error Trace:	/home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:621
          	Error:      	Received unexpected error:
          	            	Code: 12
          	            	Name: UNIMPLEMENTED
          	            	Description: Unimplemented
          	            	Details: human users may only be added by invitation
          	Test:       	TestRoleManagement/IDP_resolution
      handlers_authz.go:314: Unexpected call to *mockdb.MockStore.ListInvitationsForProject([context.Background.WithValue(jwt.userTokenContextKeyType, *openid.stdToken).WithValue(engcontext.key, *engcontext.EntityContext).WithValue(metadata.mdIncomingKey, metadata.MD) ecfe9a41-75d6-4780-a685-3f53c5df8f06]) at /home/runner/work/minder/minder/internal/controlplane/handlers_authz.go:314 because: there are no expected calls of the method "ListInvitationsForProject" for that receiver
      controller.go:251: missing call(s) to *mockdb.MockStore.GetUserBySubject(is anything, is anything) /home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:559
      controller.go:251: missing call(s) to *mockdb.MockStore.GetUserBySubject(is anything, is anything) /home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:559
      controller.go:251: aborting test due to missing call(s)
❌ TestRoleManagement/add_and_remove (1.28s)
      handlers_authz_test.go:621: 
          	Error Trace:	/home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:621
          	Error:      	Received unexpected error:
          	            	Code: 12
          	            	Name: UNIMPLEMENTED
          	            	Description: Unimplemented
          	            	Details: human users may only be added by invitation
          	Test:       	TestRoleManagement/add_and_remove
      handlers_authz_test.go:621: 
          	Error Trace:	/home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:621
          	Error:      	Received unexpected error:
          	            	Code: 12
          	            	Name: UNIMPLEMENTED
          	            	Description: Unimplemented
          	            	Details: human users may only be added by invitation
          	Test:       	TestRoleManagement/add_and_remove
      handlers_authz_test.go:626: 
          	Error Trace:	/home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:626
          	Error:      	Received unexpected error:
          	            	Code: 5
          	            	Name: NOT_FOUND
          	            	Description: Not found
          	            	Details: role assignment for this user does not exist
          	Test:       	TestRoleManagement/add_and_remove
      handlers_authz.go:314: Unexpected call to *mockdb.MockStore.ListInvitationsForProject([context.Background.WithValue(jwt.userTokenContextKeyType, *openid.stdToken).WithValue(engcontext.key, *engcontext.EntityContext).WithValue(metadata.mdIncomingKey, metadata.MD) ecfe9a41-75d6-4780-a685-3f53c5df8f06]) at /home/runner/work/minder/minder/internal/controlplane/handlers_authz.go:314 because: there are no expected calls of the method "ListInvitationsForProject" for that receiver
      controller.go:251: missing call(s) to *mockdb.MockStore.GetUserBySubject(is anything, is equal to 1ef182b1-cade-4518-9d55-ee6be3151082 (string)) /home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:559
      controller.go:251: missing call(s) to *mockdb.MockStore.GetUserBySubject(is anything, is equal to d2a4832b-acac-4be7-a408-0ad25864386a (string)) /home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:570
      controller.go:251: aborting test due to missing call(s)
❌ TestRoleManagement/simple_adds (1.62s)
      handlers_authz_test.go:621: 
          	Error Trace:	/home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:621
          	Error:      	Received unexpected error:
          	            	Code: 12
          	            	Name: UNIMPLEMENTED
          	            	Description: Unimplemented
          	            	Details: human users may only be added by invitation
          	Test:       	TestRoleManagement/simple_adds
      handlers_authz_test.go:621: 
          	Error Trace:	/home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:621
          	Error:      	Received unexpected error:
          	            	Code: 12
          	            	Name: UNIMPLEMENTED
          	            	Description: Unimplemented
          	            	Details: human users may only be added by invitation
          	Test:       	TestRoleManagement/simple_adds
      handlers_authz.go:314: Unexpected call to *mockdb.MockStore.ListInvitationsForProject([context.Background.WithValue(jwt.userTokenContextKeyType, *openid.stdToken).WithValue(engcontext.key, *engcontext.EntityContext).WithValue(metadata.mdIncomingKey, metadata.MD) ecfe9a41-75d6-4780-a685-3f53c5df8f06]) at /home/runner/work/minder/minder/internal/controlplane/handlers_authz.go:314 because: there are no expected calls of the method "ListInvitationsForProject" for that receiver
      controller.go:251: missing call(s) to *mockdb.MockStore.GetUserBySubject(is anything, is equal to 1ef182b1-cade-4518-9d55-ee6be3151082 (string)) /home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:559
      controller.go:251: missing call(s) to *mockdb.MockStore.GetUserBySubject(is anything, is equal to d2a4832b-acac-4be7-a408-0ad25864386a (string)) /home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:559
      controller.go:251: aborting test due to missing call(s)

evankanderson · 2025-11-19T20:41:11Z

Oh, there's also direct access to this flag not through a constant in the test here, at least: https://github.com/mindersec/minder/blob/main/internal/controlplane/handlers_authz_test.go#L577

Signed-off-by: Mujib Ahasan <[email protected]>

Mujib-Ahasan · 2025-11-22T11:15:16Z

Hey @evankanderson, few changes have been made to normalize those errors, but after that I got few new unwanted errors!! I tried to solve them too but did not work out. These new sets of error occurred because of the mismatch of json. Is there any way I could bypass them?

--- FAIL: TestRoleManagement (0.00s)
    --- FAIL: TestRoleManagement/simple_adds (0.02s)
        c:\dev\minder\internal\controlplane\handlers_authz_test.go:644: 
            	Error Trace:	c:/dev/minder/internal/controlplane/handlers_authz_test.go:644
            	Error:      	elements differ
            	            	
            	            	extra elements in list A:
            	            	([]interface {}) (len=2) {
            	            	 (string) (len=139) "{\"role\":\"admin\", \"subject\":\"61ab1775-3a20-4406-a37d-3eb8d0aa30d3\", \"displayName\":\"user1\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}",
            	            	 (string) (len=139) "{\"role\":\"admin\", \"subject\":\"6c0c62e4-002d-4f0b-9c44-8a3673927846\", \"displayName\":\"user2\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}"
            	            	}
            	            	
            	            	
            	            	listA:
            	            	([]string) (len=2) {
            	            	 (string) (len=139) "{\"role\":\"admin\", \"subject\":\"61ab1775-3a20-4406-a37d-3eb8d0aa30d3\", \"displayName\":\"user1\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}",
            	            	 (string) (len=139) "{\"role\":\"admin\", \"subject\":\"6c0c62e4-002d-4f0b-9c44-8a3673927846\", \"displayName\":\"user2\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}"
            	            	}
            	            	
            	            	
            	            	listB:
            	            	([]string) {
            	            	}
            	Test:       	TestRoleManagement/simple_adds
            	Messages:   	RPC results mismatch, want: A, got: B
        c:\dev\minder\internal\controlplane\handlers_authz_test.go:653: 
            	Error Trace:	c:/dev/minder/internal/controlplane/handlers_authz_test.go:653
            	Error:      	elements differ
            	            	
            	            	extra elements in list A:
            	            	([]interface {}) (len=2) {
            	            	 (string) (len=116) "{\"role\":\"admin\", \"subject\":\"61ab1775-3a20-4406-a37d-3eb8d0aa30d3\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}",
            	            	 (string) (len=116) "{\"role\":\"admin\", \"subject\":\"6c0c62e4-002d-4f0b-9c44-8a3673927846\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}"
            	            	}
            	            	
            	            	
            	            	listA:
            	            	([]string) (len=2) {
            	            	 (string) (len=116) "{\"role\":\"admin\", \"subject\":\"61ab1775-3a20-4406-a37d-3eb8d0aa30d3\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}",
            	            	 (string) (len=116) "{\"role\":\"admin\", \"subject\":\"6c0c62e4-002d-4f0b-9c44-8a3673927846\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}"
            	            	}
            	            	
            	            	
            	            	listB:
            	            	([]string) {
            	            	}
            	Test:       	TestRoleManagement/simple_adds
            	Messages:   	Stored results mismatch, want: A, got: B
    --- FAIL: TestRoleManagement/add_and_remove (0.04s)
        c:\dev\minder\internal\controlplane\handlers_authz_test.go:644: 
            	Error Trace:	c:/dev/minder/internal/controlplane/handlers_authz_test.go:644
            	Error:      	elements differ
            	            	
            	            	extra elements in list A:
            	            	([]interface {}) (len=1) {
            	            	 (string) (len=139) "{\"role\":\"admin\", \"subject\":\"61ab1775-3a20-4406-a37d-3eb8d0aa30d3\", \"displayName\":\"user1\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}"
            	            	}
            	            	
            	            	
            	            	listA:
            	            	([]string) (len=1) {
            	            	 (string) (len=139) "{\"role\":\"admin\", \"subject\":\"61ab1775-3a20-4406-a37d-3eb8d0aa30d3\", \"displayName\":\"user1\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}"
            	            	}
            	            	
            	            	
            	            	listB:
            	            	([]string) {
            	            	}
            	Test:       	TestRoleManagement/add_and_remove
            	Messages:   	RPC results mismatch, want: A, got: B
    --- FAIL: TestRoleManagement/IDP_resolution (0.09s)
        c:\dev\minder\internal\controlplane\handlers_authz_test.go:644: 
            	Error Trace:	c:/dev/minder/internal/controlplane/handlers_authz_test.go:644
            	Error:      	elements differ
            	            	
            	            	extra elements in list A:
            	            	([]interface {}) (len=2) {
            	            	 (string) (len=139) "{\"role\":\"admin\", \"subject\":\"61ab1775-3a20-4406-a37d-3eb8d0aa30d3\", \"displayName\":\"user1\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}",
            	            	 (string) (len=139) "{\"role\":\"admin\", \"subject\":\"6c0c62e4-002d-4f0b-9c44-8a3673927846\", \"displayName\":\"user2\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}"
            	            	}
            	            	
            	            	
            	            	listA:
            	            	([]string) (len=2) {
            	            	 (string) (len=139) "{\"role\":\"admin\", \"subject\":\"61ab1775-3a20-4406-a37d-3eb8d0aa30d3\", \"displayName\":\"user1\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}",
            	            	 (string) (len=139) "{\"role\":\"admin\", \"subject\":\"6c0c62e4-002d-4f0b-9c44-8a3673927846\", \"displayName\":\"user2\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}"
            	            	}
            	            	
            	            	
            	            	listB:
            	            	([]string) {
            	            	}
            	Test:       	TestRoleManagement/IDP_resolution
            	Messages:   	RPC results mismatch, want: A, got: B
        c:\dev\minder\internal\controlplane\handlers_authz_test.go:653: 
            	Error Trace:	c:/dev/minder/internal/controlplane/handlers_authz_test.go:653
            	Error:      	elements differ
            	            	
            	            	extra elements in list A:
            	            	([]interface {}) (len=2) {
            	            	 (string) (len=116) "{\"role\":\"admin\", \"subject\":\"61ab1775-3a20-4406-a37d-3eb8d0aa30d3\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}",
            	            	 (string) (len=116) "{\"role\":\"admin\", \"subject\":\"6c0c62e4-002d-4f0b-9c44-8a3673927846\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}"
            	            	}
            	            	
            	            	
            	            	listA:
            	            	([]string) (len=2) {
            	            	 (string) (len=116) "{\"role\":\"admin\", \"subject\":\"61ab1775-3a20-4406-a37d-3eb8d0aa30d3\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}",
            	            	 (string) (len=116) "{\"role\":\"admin\", \"subject\":\"6c0c62e4-002d-4f0b-9c44-8a3673927846\", \"project\":\"25ad6f5d-ad68-42ff-b8a1-c15a596fe03b\"}"
            	            	}
            	            	
            	            	
            	            	listB:
            	            	([]string) {
            	            	}
            	Test:       	TestRoleManagement/IDP_resolution
            	Messages:   	Stored results mismatch, want: A, got: B
FAIL
FAIL	github.com/mindersec/minder/internal/controlplane	0.275s
FAIL

evankanderson · 2025-11-30T22:51:06Z

Sorry for the delay in responding; I was travelling for Thanksgiving, and needed to actually clone and run your code to see what's going on.

A bit of history here:

Originally, Minder didn't have any user management features. The first iteration of user management was added by engineering effectively as a test of the integration with OpenFGA. In particular, you needed to add users by their IDP sub value, which was a UUID which you could only find out by asking the corresponding user to log in and then copy-paste it back to you.
Next, engineering implemented the ability to look up by display name, which made it possible to directly add users to projects if they had already logged in to Minder (and created a default project of their own). This was still a direct add, in that the recipient didn't have a way to prevent being added to the project.
After looking at the implementation, product management pointed out two problems:
1. There was no way to add a user who hadn't already logged in to Minder
2. Users didn't have an opportunity to consent to being added to a project.
The solution to both of these was the user_management feature which added the ability to send (email) invites to users. Given the problems in (2), we decided that if email invites were enabled, the direct-add via username or sub value should be disabled.

The test you're struggling with was written for the "direct add" functionality. You may want to replace the direct adds in Subject with Email adds, and then you'll need to set up the corresponding context when they call server.ResolveInvitation. Let me know if you want some more help with that, or if that's enough to get you going again.

evankanderson · 2025-12-01T16:37:07Z

If it would help, I could write a fake for InvitationService that had the same interfaces as InvitationService, but stored the status in-memory, rather than needing to line up mock calls and responses.

Remove: user_management feature flag

8105530

Signed-off-by: Mujib Ahasan <[email protected]>

Mujib-Ahasan requested a review from a team as a code owner November 13, 2025 17:39

Merge branch 'main' into release-userManagement-flag

e591db3

evankanderson reviewed Nov 13, 2025

View reviewed changes

Mujib-Ahasan added 2 commits November 14, 2025 15:16

update: more dead codes removed

f104d5b

Signed-off-by: Mujib Ahasan <[email protected]>

Merge branch 'main' into release-userManagement-flag

733a02b

Update test cases

c0655a5

Signed-off-by: Mujib Ahasan <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Removed: user_management feature flag #5970

Removed: user_management feature flag #5970

Mujib-Ahasan commented Nov 13, 2025

Uh oh!

Mujib-Ahasan commented Nov 13, 2025

Uh oh!

evankanderson left a comment

Uh oh!

evankanderson Nov 13, 2025

Uh oh!

Mujib-Ahasan Nov 14, 2025

Uh oh!

evankanderson Nov 13, 2025

Uh oh!

evankanderson Nov 13, 2025

Uh oh!

evankanderson commented Nov 19, 2025

Uh oh!

evankanderson commented Nov 19, 2025

Uh oh!

Mujib-Ahasan commented Nov 22, 2025 •

edited

Loading

Uh oh!

evankanderson commented Nov 30, 2025

Uh oh!

evankanderson commented Dec 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Removed: user_management feature flag #5970

Are you sure you want to change the base?

Removed: user_management feature flag #5970

Conversation

Mujib-Ahasan commented Nov 13, 2025

Summary

Uh oh!

Mujib-Ahasan commented Nov 13, 2025

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

evankanderson Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Mujib-Ahasan Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

evankanderson Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

evankanderson Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

evankanderson commented Nov 19, 2025

Uh oh!

evankanderson commented Nov 19, 2025

Uh oh!

Mujib-Ahasan commented Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

evankanderson commented Nov 30, 2025

Uh oh!

evankanderson commented Dec 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Mujib-Ahasan commented Nov 22, 2025 •

edited

Loading