Support user-auth service principal #68
Conversation
- If no user present OR no ID token AND usernameAssertion is disallowed - Update tests
|
We need to have design discussion regarding this |
There was a problem hiding this comment.
AzureSeetings need a new switch to allow/disallow usage of the service credentials.
I would move the fallback logic directly into the user token provider. It needs to be redesigned somehow. Probably should contain an instance of service token provider internally and delegate access token request to it. e.g. work as a composite token provider (a decorator).
azcredentials/credentials.go
Outdated
| @@ -14,6 +14,7 @@ type AzureCredentials interface { | |||
|
|
|||
| // AadCurrentUserCredentials "Current User" user identity credentials of the current Grafana user. | |||
| type AadCurrentUserCredentials struct { | |||
| ServicePrincipal AzureClientSecretCredentials | |||
There was a problem hiding this comment.
- Maybe let's we call it 'ServiceCredentials' to not introduce new terminology?
- Let's not limit it to
AzureClientSecretCredentials, since there's no functional difference between which type of credentials is going to be used as long as it's service credentials. I would not bother to define new type hierarchy for "service" credentials specifically. Would just use "AzureCredentials" and have runtime check that they are not user identity. - It should be optional, I think it needs to be a pointer (or once it's changed to interface
AzureCredentialsit will be fine).
There was a problem hiding this comment.
I've made changes here that should meet the requirements.
| @@ -71,6 +71,14 @@ func NewAzureAccessTokenProvider(settings *azsettings.AzureSettings, credentials | |||
| err = fmt.Errorf("user identity authentication is not enabled in Grafana config") | |||
| return nil, err | |||
| } | |||
|
|
|||
| var tokenRetriever TokenRetriever | |||
There was a problem hiding this comment.
I would rather move the logic upper, before the switch (line 34) and wrote something like this (pseudocode):
nonUserContext = <determine from user context> // i.e. not user initiated, no user context
if isUserIdentityCredentials(*credentials) && nonUserContext {
if credentials.ServiceCredentials != nils {
credentials = credentials.ServiceCredentials
} else {
error = "Attempt to user user credentials in non-user context"
}
}There was a problem hiding this comment.
OBO is also user identity authentication
There was a problem hiding this comment.
OK, I see, user context isn't available in NewAzureAccessTokenProvider. It needs to be done inside userTokenProvider. userTokenProvider can be a decorator for service token provider.
# Conflicts: # azsettings/settings_test.go
aangelisc
added
enhancement
This PR updates the SDK to support a service principal credential when using user-authentication methods. This will allow us to support backend functionality via the service principal credentials which will always be present.
Changes:
ServiceCredentialsandServiceCredentialsEnabledproperties added to current user credentials structUserIdentityFallbackCredentialsEnabledsetting. Enabling this will allow users to specify fallback credentials when creating user identity data sources. This defaults totrue.CurrentUserContextstruct and request functions to appropriately setGrafanaIdTokenIt should be noted that for backend requests to be identified accurately the feature toggle
idForwardingshould be enabled.Part of grafana/grafana#81918
Fixes #122