stacklok
Removed: user_management feature flag
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
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??
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)
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
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
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
subvalue, 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:
- There was no way to add a user who hadn't already logged in to Minder
- Users didn't have an opportunity to consent to being added to a project.
The solution to both of these was the
user_managementfeature 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 orsubvalue 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.
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.
Hello @evankanderson, I am kinda stuck and have been struggling with test cases for days. Your directions are very clear, I have to enable Email adds instead of direct adds. On that in my mind I tried to make some changes(like- replaced Subject with Email) with different combination, but failed. Could you please help how this changes should be made.( sorry for late responses, exam were going on)