ClientCredentials flow in basic User operations (createUser, getUser, resendInvitationMail) #529
Conversation
adrianlop
force-pushed
the
Clients_create_users
branch
3 times, most recently
from
ec35029 to
f463135
Compare
| @@ -92,7 +94,7 @@ func (s *UserMgmtServer) authAPIHandle(handle authedHandle, requiresAdmin bool) | |||
| s.writeError(w, err) | |||
| return | |||
| } | |||
| if creds.User.Disabled || (requiresAdmin && !creds.User.Admin) { | |||
| if creds.User.ID != "" && (creds.User.Disabled || (requiresAdmin && !creds.User.Admin)) { | |||
There was a problem hiding this comment.
creds.User.ID != "" shouldn't be checked here. There should be two paths for this code.
if clientcred {
} else {
}
f463135 to
085978d
Compare
|
PR updated. The flag is a hard choice now, it forces you to create users only using client credentials token, if enabled. |
adrianlop
changed the title
adrianlop
force-pushed
the
Clients_create_users
branch
from
085978d to
821e3f0
Compare
|
updated PR. now it supports createUser, getUser, resendInvitationMail, not only createUser. |
| @@ -51,6 +51,9 @@ func main() { | |||
|
|
|||
| enableClientRegistration := fs.Bool("enable-client-registration", false, "Allow dynamic registration of clients") | |||
|
|
|||
| // Client credentials administration | |||
| forceClientUserCreation := fs.Bool("force-client-user-creation", false, "force create users using admin client-credentials") | |||
There was a problem hiding this comment.
This isn't just about client creation, it's about the entire layer of API access.
Let's do --api-use-client-credentials
adrianlop
force-pushed
the
Clients_create_users
branch
from
821e3f0 to
721a60d
Compare
|
thanks, PR updated with your comments. |
| @@ -51,6 +51,9 @@ func main() { | |||
|
|
|||
| enableClientRegistration := fs.Bool("enable-client-registration", false, "Allow dynamic registration of clients") | |||
|
|
|||
| // Client credentials administration | |||
| apiUseClientCredentials := fs.Bool("api-use-client-credentials", false, "Forces API to authenticate using client credentials instead of an ID token.Clients must be 'admin clients' to use the API.") | |||
There was a problem hiding this comment.
s/token.Clients/token. Clients/g
|
this looks good to me, but requires fixing the spelling mistakes in the command line flag and adding some unit/integration tests. |
|
ok! I'm working on the tests, will update the PR when ready. |
adrianlop
force-pushed
the
Clients_create_users
branch
from
721a60d to
2a4729e
Compare
|
PR updated with tests! |
| @@ -869,7 +988,7 @@ func (t *testEmailer) SendInviteEmail(email string, redirectURL url.URL, clientI | |||
| return retURL, nil | |||
| } | |||
|
|
|||
| func makeUserToken(issuerURL url.URL, userID, clientID string, expires time.Duration, privKey *key.PrivateKey) string { | |||
| func makeToken(issuerURL url.URL, userID, clientID string, expires time.Duration, privKey *key.PrivateKey) string { | |||
There was a problem hiding this comment.
in the future please don't include changes that aren't relevant to the pr.
There was a problem hiding this comment.
ah my bad, I see why you did this. perhaps create a different method that doesn't take a clientID instead?
|
two more nits, sorry. |
adrianlop
force-pushed
the
Clients_create_users
branch
from
2a4729e to
9b8ab3b
Compare
|
sure no prob, PR updated with your comments. |
|
lgtm! |
Fixes #528