From 35e337718799e9167db96c17756db38f64bbad27 Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Thu, 12 Oct 2023 16:43:39 +0200 Subject: [PATCH 1/2] feat: use OIDC ID token for Azure --- internal/api/external_azure_test.go | 151 +++++++++++++++++++++++----- internal/api/external_test.go | 4 +- internal/api/provider/azure.go | 134 +++++++++++++++--------- internal/api/provider/azure_test.go | 29 ++++++ internal/api/provider/oidc.go | 128 ++++++++++++++++++++++- internal/api/provider/oidc_test.go | 102 +++++++++++++++++++ 6 files changed, 474 insertions(+), 74 deletions(-) create mode 100644 internal/api/provider/azure_test.go diff --git a/internal/api/external_azure_test.go b/internal/api/external_azure_test.go index db8edc59e..0eedc0887 100644 --- a/internal/api/external_azure_test.go +++ b/internal/api/external_azure_test.go @@ -1,12 +1,22 @@ package api import ( + "context" + "crypto" + "crypto/rsa" + "crypto/sha256" + "crypto/x509" + "encoding/base64" + "encoding/json" "fmt" "net/http" "net/http/httptest" "net/url" + "time" + "github.com/coreos/go-oidc/v3/oidc" jwt "github.com/golang-jwt/jwt" + "github.com/supabase/gotrue/internal/api/provider" ) const ( @@ -14,6 +24,84 @@ const ( azureUserNoEmail string = `{"name":"Azure Test","sub":"azuretestid"}` ) +func idTokenPrivateKey() *rsa.PrivateKey { + // #nosec + der, err := base64.StdEncoding.DecodeString("MIIEpAIBAAKCAQEAvklrFDsVgbhs3DOQICMqm4xdFoi/MHj/T6XH8S7wXWd0roqdWVarwCLV4y3DILkLre4PzNK+hEY5NAnoAKrsCMyyCb4Wdl8HCdJk4ojDqAig+DJw67imqZoxJMFJyIhfMJhwVK1V8GRUPATn855rygLo7wThahMJeEHNiJr3TtV6Rf35KSs7DuyoWIUSjISYabQozKqIvpdUpTpSqjlOQvjdAxggRyycBZSgLzjWhsA8metnAMO48bX4bgiHLR6Kzu/dfPyEVPfgeYpA2ebIY6GzIUxVS0yX8+ExA6jeLCkuepjLHuz5XCJtd6zzGDXr1eX7nA6ZIeUNdFbWRDnPawIDAQABAoIBABH4Qvl1HvHSJc2hvPGcAJER71SKc2uzcYDnCfu30BEyDO3Sv0tJiQyq/YHnt26mqviw66MPH9jD/PDyIou1mHa4RfPvlJV3IeYGjWprOfbrYbAuq0VHec24dv2el0YtwreHHcyRVfVOtDm6yODTzCAWqEKyNktbIuDNbgiBgetayaJecDRoFMF9TOCeMCL92iZytzAr7fi+JWtLkRS/GZRIBjbr8LJ/ueYoCRmIx3MIw0WdPp7v2ZfeRTxP7LxJZ+MAsrq2pstmZYP7K0305e0bCJX1HexfXLs2Ul7u8zaxrXL8zw4/9+/GMsAeU3ffCVnGz/RKL5+T6iuz2RotjFECgYEA+Xk7DGwRXfDg9xba1GVFGeiC4nybqZw/RfZKcz/RRJWSHRJV/ps1avtbca3B19rjI6rewZMO1NWNv/tI2BdXP8vAKUnI9OHJZ+J/eZzmqDE6qu0v0ddRFUDzCMWE0j8BjrUdy44n4NQgopcv14u0iyr9tuhGO6YXn2SuuvEkZokCgYEAw0PNnT55kpkEhXSp7An2hdBJEub9ST7hS6Kcd8let62/qUZ/t5jWigSkWC1A2bMtH55+LgudIFjiehwVzRs7jym2j4jkKZGonyAX1l9IWgXwKl7Pn49lEQH5Yk6MhnXdyLGoFTzXiUyk/fKvgXX7jow1bD3j6sAc8P495I7TyVMCgYAHg6VJrH+har37805IE3zPWPeIRuSRaUlmnBKGAigVfsPV6FV6w8YKIOQSOn+aNtecnWr0Pa+2rXAFllYNXDaej06Mb9KDvcFJRcM9MIKqEkGIIHjOQ0QH9drcKsbjZk5vs/jfxrpgxULuYstoHKclgff+aGSlK02O2YOB0f2csQKBgQCEC/MdNiWCpKXxFg7fB3HF1i/Eb56zjKlQu7uyKeQ6tG3bLEisQNg8Z5034Apt7gRC0KyluMbeHB2z1BBOLu9dBill8X3SOqVcTpiwKKlF76QVEx622YLQOJSMDXBscYK0+KchDY74U3N0JEzZcI7YPCrYcxYRJy+rLVNvn8LK7wKBgQDE8THsZ589e10F0zDBvPK56o8PJnPeH71sgdM2Co4oLzBJ6g0rpJOKfcc03fLHsoJVOAya9WZeIy6K8+WVdcPTadR07S4p8/tcK1eguu5qlmCUOzswrTKAaJoIHO7cddQp3nySIqgYtkGdHKuvlQDMQkEKJS0meOm+vdeAG2rkaA==") + if err != nil { + panic(err) + } + + privateKey, err := x509.ParsePKCS1PrivateKey(der) + if err != nil { + panic(err) + } + + privateKey.E = 65537 + + return privateKey +} + +func setupAzureOverrideVerifiers() { + provider.OverrideVerifiers["https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/oauth2/v2.0/authorize"] = func(ctx context.Context, config *oidc.Config) *oidc.IDTokenVerifier { + pk := idTokenPrivateKey() + + return oidc.NewVerifier( + provider.IssuerAzureMicrosoft, + &oidc.StaticKeySet{ + PublicKeys: []crypto.PublicKey{ + &pk.PublicKey, + }, + }, + config, + ) + } +} + +func mintIDToken(user string) string { + var idToken struct { + Issuer string `json:"iss"` + IssuedAt int `json:"iat"` + ExpiresAt int `json:"exp"` + Audience string `json:"aud"` + + Sub string `json:"sub,omitempty"` + Name string `json:"name,omitempty"` + Email string `json:"email,omitempty"` + XmsEdov any `json:"xms_edov,omitempty"` + } + + if err := json.Unmarshal([]byte(user), &idToken); err != nil { + panic(err) + } + + now := time.Now() + + idToken.Issuer = provider.IssuerAzureMicrosoft + idToken.IssuedAt = int(now.Unix()) + idToken.ExpiresAt = int(now.Unix() + 60*60) + idToken.Audience = "testclientid" + + header := base64.RawURLEncoding.EncodeToString([]byte(`{"typ":"JWT","alg":"RS256"}`)) + + data, err := json.Marshal(idToken) + if err != nil { + panic(err) + } + + payload := base64.RawURLEncoding.EncodeToString(data) + sum := sha256.Sum256([]byte(header + "." + payload)) + + pk := idTokenPrivateKey() + sig, err := rsa.SignPKCS1v15(nil, pk, crypto.SHA256, sum[:]) + if err != nil { + panic(err) + } + + token := header + "." + payload + "." + base64.RawURLEncoding.EncodeToString(sig) + + return token +} + func (ts *ExternalTestSuite) TestSignupExternalAzure() { req := httptest.NewRequest(http.MethodGet, "http://localhost/authorize?provider=azure", nil) w := httptest.NewRecorder() @@ -38,7 +126,7 @@ func (ts *ExternalTestSuite) TestSignupExternalAzure() { ts.Equal(ts.Config.SiteURL, claims.SiteURL) } -func AzureTestSignupSetup(ts *ExternalTestSuite, tokenCount *int, userCount *int, code string, user string) *httptest.Server { +func AzureTestSignupSetup(ts *ExternalTestSuite, tokenCount *int, code string, user string) *httptest.Server { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case "/oauth2/v2.0/token": @@ -48,11 +136,7 @@ func AzureTestSignupSetup(ts *ExternalTestSuite, tokenCount *int, userCount *int ts.Equal(ts.Config.External.Azure.RedirectURI, r.FormValue("redirect_uri")) w.Header().Add("Content-Type", "application/json") - fmt.Fprint(w, `{"access_token":"azure_token","expires_in":100000}`) - case "/oidc/userinfo": - *userCount++ - w.Header().Add("Content-Type", "application/json") - fmt.Fprint(w, user) + fmt.Fprintf(w, `{"access_token":"azure_token","expires_in":100000,"id_token":%q}`, mintIDToken(user)) default: w.WriteHeader(500) ts.Fail("unknown azure oauth call %s", r.URL.Path) @@ -66,22 +150,26 @@ func AzureTestSignupSetup(ts *ExternalTestSuite, tokenCount *int, userCount *int } func (ts *ExternalTestSuite) TestSignupExternalAzure_AuthorizationCode() { + setupAzureOverrideVerifiers() + ts.Config.DisableSignup = false - tokenCount, userCount := 0, 0 + tokenCount := 0 code := "authcode" - server := AzureTestSignupSetup(ts, &tokenCount, &userCount, code, azureUser) + server := AzureTestSignupSetup(ts, &tokenCount, code, azureUser) defer server.Close() u := performAuthorization(ts, "azure", code, "") - assertAuthorizationSuccess(ts, u, tokenCount, userCount, "azure@example.com", "Azure Test", "azuretestid", "") + assertAuthorizationSuccess(ts, u, tokenCount, -1, "azure@example.com", "Azure Test", "azuretestid", "") } func (ts *ExternalTestSuite) TestSignupExternalAzureDisableSignupErrorWhenNoUser() { + setupAzureOverrideVerifiers() + ts.Config.DisableSignup = true - tokenCount, userCount := 0, 0 + tokenCount := 0 code := "authcode" - server := AzureTestSignupSetup(ts, &tokenCount, &userCount, code, azureUser) + server := AzureTestSignupSetup(ts, &tokenCount, code, azureUser) defer server.Close() u := performAuthorization(ts, "azure", code, "") @@ -90,52 +178,59 @@ func (ts *ExternalTestSuite) TestSignupExternalAzureDisableSignupErrorWhenNoUser } func (ts *ExternalTestSuite) TestSignupExternalAzureDisableSignupErrorWhenNoEmail() { + setupAzureOverrideVerifiers() + ts.Config.DisableSignup = true - tokenCount, userCount := 0, 0 + tokenCount := 0 code := "authcode" - server := AzureTestSignupSetup(ts, &tokenCount, &userCount, code, azureUserNoEmail) + server := AzureTestSignupSetup(ts, &tokenCount, code, azureUserNoEmail) defer server.Close() u := performAuthorization(ts, "azure", code, "") assertAuthorizationFailure(ts, u, "Error getting user email from external provider", "server_error", "azure@example.com") - } func (ts *ExternalTestSuite) TestSignupExternalAzureDisableSignupSuccessWithPrimaryEmail() { + setupAzureOverrideVerifiers() + ts.Config.DisableSignup = true ts.createUser("azuretestid", "azure@example.com", "Azure Test", "http://example.com/avatar", "") - tokenCount, userCount := 0, 0 + tokenCount := 0 code := "authcode" - server := AzureTestSignupSetup(ts, &tokenCount, &userCount, code, azureUser) + server := AzureTestSignupSetup(ts, &tokenCount, code, azureUser) defer server.Close() u := performAuthorization(ts, "azure", code, "") - assertAuthorizationSuccess(ts, u, tokenCount, userCount, "azure@example.com", "Azure Test", "azuretestid", "http://example.com/avatar") + assertAuthorizationSuccess(ts, u, tokenCount, -1, "azure@example.com", "Azure Test", "azuretestid", "http://example.com/avatar") } func (ts *ExternalTestSuite) TestInviteTokenExternalAzureSuccessWhenMatchingToken() { + setupAzureOverrideVerifiers() + // name should be populated from Azure API ts.createUser("azuretestid", "azure@example.com", "", "http://example.com/avatar", "invite_token") - tokenCount, userCount := 0, 0 + tokenCount := 0 code := "authcode" - server := AzureTestSignupSetup(ts, &tokenCount, &userCount, code, azureUser) + server := AzureTestSignupSetup(ts, &tokenCount, code, azureUser) defer server.Close() u := performAuthorization(ts, "azure", code, "invite_token") - assertAuthorizationSuccess(ts, u, tokenCount, userCount, "azure@example.com", "Azure Test", "azuretestid", "http://example.com/avatar") + assertAuthorizationSuccess(ts, u, tokenCount, -1, "azure@example.com", "Azure Test", "azuretestid", "http://example.com/avatar") } func (ts *ExternalTestSuite) TestInviteTokenExternalAzureErrorWhenNoMatchingToken() { - tokenCount, userCount := 0, 0 + setupAzureOverrideVerifiers() + + tokenCount := 0 code := "authcode" azureUser := `{"name":"Azure Test","avatar":{"href":"http://example.com/avatar"}}` - server := AzureTestSignupSetup(ts, &tokenCount, &userCount, code, azureUser) + server := AzureTestSignupSetup(ts, &tokenCount, code, azureUser) defer server.Close() w := performAuthorizationRequest(ts, "azure", "invite_token") @@ -143,12 +238,14 @@ func (ts *ExternalTestSuite) TestInviteTokenExternalAzureErrorWhenNoMatchingToke } func (ts *ExternalTestSuite) TestInviteTokenExternalAzureErrorWhenWrongToken() { + setupAzureOverrideVerifiers() + ts.createUser("azuretestid", "azure@example.com", "", "", "invite_token") - tokenCount, userCount := 0, 0 + tokenCount := 0 code := "authcode" azureUser := `{"name":"Azure Test","avatar":{"href":"http://example.com/avatar"}}` - server := AzureTestSignupSetup(ts, &tokenCount, &userCount, code, azureUser) + server := AzureTestSignupSetup(ts, &tokenCount, code, azureUser) defer server.Close() w := performAuthorizationRequest(ts, "azure", "wrong_token") @@ -156,12 +253,14 @@ func (ts *ExternalTestSuite) TestInviteTokenExternalAzureErrorWhenWrongToken() { } func (ts *ExternalTestSuite) TestInviteTokenExternalAzureErrorWhenEmailDoesntMatch() { + setupAzureOverrideVerifiers() + ts.createUser("azuretestid", "azure@example.com", "", "", "invite_token") - tokenCount, userCount := 0, 0 + tokenCount := 0 code := "authcode" azureUser := `{"name":"Azure Test", "email":"other@example.com", "avatar":{"href":"http://example.com/avatar"}}` - server := AzureTestSignupSetup(ts, &tokenCount, &userCount, code, azureUser) + server := AzureTestSignupSetup(ts, &tokenCount, code, azureUser) defer server.Close() u := performAuthorization(ts, "azure", code, "invite_token") diff --git a/internal/api/external_test.go b/internal/api/external_test.go index be182e2f2..2dacd2dbe 100644 --- a/internal/api/external_test.go +++ b/internal/api/external_test.go @@ -160,7 +160,9 @@ func assertAuthorizationSuccess(ts *ExternalTestSuite, u *url.URL, tokenCount in ts.Equal("bearer", v.Get("token_type")) ts.Equal(1, tokenCount) - ts.Equal(1, userCount) + if userCount > -1 { + ts.Equal(1, userCount) + } // ensure user has been created with metadata user, err := models.FindUserByEmailAndAudience(ts.API.db, email, ts.Config.JWT.Aud) diff --git a/internal/api/provider/azure.go b/internal/api/provider/azure.go index 21ab0098b..1c456980a 100644 --- a/internal/api/provider/azure.go +++ b/internal/api/provider/azure.go @@ -2,30 +2,45 @@ package provider import ( "context" - "errors" + "encoding/base64" + "encoding/json" + "fmt" + "regexp" "strings" + "github.com/coreos/go-oidc/v3/oidc" "github.com/supabase/gotrue/internal/conf" "golang.org/x/oauth2" ) const IssuerAzure = "https://login.microsoftonline.com/common/v2.0" +// IssuerAzureMicrosoft is the OIDC issuer for microsoft.com accounts: +// https://learn.microsoft.com/en-us/azure/active-directory/develop/id-token-claims-reference#payload-claims +const IssuerAzureMicrosoft = "https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0" + const ( defaultAzureAuthBase = "login.microsoftonline.com/common" - defaultAzureAPIBase = "graph.microsoft.com" ) type azureProvider struct { *oauth2.Config - APIPath string + + // ExpectedIssuer contains the OIDC issuer that should be expected when + // the authorize flow completes. For example, when using the "common" + // endpoint the authorization flow will end with an ID token that + // contains any issuer. In this case, ExpectedIssuer is an empty + // string, because any issuer is allowed. But if a developer sets up a + // tenant-specific authorization endpoint, then we must ensure that the + // ID token received is issued by that specific issuer, and so + // ExpectedIssuer contains the issuer URL of that tenant. + ExpectedIssuer string } -type azureUser struct { - Name string `json:"name"` - Email string `json:"email"` - Sub string `json:"sub"` - OtherMails []string `json:"otherMails"` +var azureIssuerRegexp = regexp.MustCompile("^https://login[.]microsoftonline[.]com/([^/]+)/v2[.]0/?$") + +func IsAzureIssuer(issuer string) bool { + return azureIssuerRegexp.MatchString(issuer) } // NewAzureProvider creates a Azure account provider. @@ -34,15 +49,25 @@ func NewAzureProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuth return nil, err } - authHost := chooseHost(ext.URL, defaultAzureAuthBase) - apiPath := chooseHost(ext.ApiURL, defaultAzureAPIBase) - oauthScopes := []string{"openid"} if scopes != "" { oauthScopes = append(oauthScopes, strings.Split(scopes, ",")...) } + authHost := chooseHost(ext.URL, defaultAzureAuthBase) + expectedIssuer := "" + + if ext.URL != "" { + expectedIssuer = authHost + "/v2.0" + + if !IsAzureIssuer(expectedIssuer) { + // in tests, the URL is a local server which should not + // be the expected issuer + expectedIssuer = "" + } + } + return &azureProvider{ Config: &oauth2.Config{ ClientID: ext.ClientID[0], @@ -54,7 +79,7 @@ func NewAzureProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuth RedirectURL: ext.RedirectURI, Scopes: oauthScopes, }, - APIPath: apiPath, + ExpectedIssuer: expectedIssuer, }, nil } @@ -62,50 +87,67 @@ func (g azureProvider) GetOAuthToken(code string) (*oauth2.Token, error) { return g.Exchange(context.Background(), code) } -func (g azureProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (*UserProvidedData, error) { - var u azureUser - if err := makeRequest(ctx, tok, g.Config, g.APIPath+"/oidc/userinfo", &u); err != nil { - return nil, err +func (g azureProvider) detectIDTokenIssuer(ctx context.Context, idToken string) (string, error) { + var payload struct { + Issuer string `json:"iss"` } - var data UserProvidedData - - data.Metadata = &Claims{ - Issuer: g.APIPath, - Subject: u.Sub, - Name: u.Name, + parts := strings.Split(idToken, ".") + if len(parts) != 3 { + return "", fmt.Errorf("azure: invalid ID token") + } - // To be deprecated - FullName: u.Name, - ProviderId: u.Sub, + payloadBytes, err := base64.RawURLEncoding.DecodeString(parts[1]) + if err != nil { + return "", fmt.Errorf("azure: invalid ID token %w", err) } - if u.Email != "" { - data.Emails = append(data.Emails, Email{ - Email: u.Email, - Verified: true, - }) + if err := json.Unmarshal(payloadBytes, &payload); err != nil { + return "", fmt.Errorf("azure: invalid ID token %w", err) } - if u.OtherMails != nil { - for _, mail := range u.OtherMails { - if mail != "" { - data.Emails = append(data.Emails, Email{ - Email: mail, - Verified: false, - }) - } + return payload.Issuer, nil +} + +func (g azureProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (*UserProvidedData, error) { + idToken := tok.Extra("id_token") + + if idToken != nil { + issuer, err := g.detectIDTokenIssuer(ctx, idToken.(string)) + if err != nil { + return nil, err } - } - if len(data.Emails) == 0 { - return nil, errors.New("unable to find email with Azure provider") - } + if !IsAzureIssuer(issuer) { + return nil, fmt.Errorf("azure: ID token issuer not valid %q", issuer) + } - data.Emails[0].Primary = true + if g.ExpectedIssuer != "" && issuer != g.ExpectedIssuer { + // Since ExpectedIssuer was set, then the developer had + // setup GoTrue to use the tenant-specific + // authorization endpoint, which in-turn means that + // only those tenant's ID tokens will be accepted. + return nil, fmt.Errorf("azure: ID token issuer %q does not match expected issuer %q", issuer, g.ExpectedIssuer) + } + + provider, err := oidc.NewProvider(ctx, issuer) + if err != nil { + return nil, err + } + + _, data, err := ParseIDToken(ctx, provider, &oidc.Config{ + ClientID: g.ClientID, + }, idToken.(string), ParseIDTokenOptions{ + AccessToken: tok.AccessToken, + }) + if err != nil { + return nil, err + } + + return data, nil + } - data.Metadata.Email = data.Emails[0].Email - data.Metadata.EmailVerified = data.Emails[0].Verified + // Only ID tokens supported, UserInfo endpoint has a history of being less secure. - return &data, nil + return nil, fmt.Errorf("azure: no OIDC ID token present in response") } diff --git a/internal/api/provider/azure_test.go b/internal/api/provider/azure_test.go new file mode 100644 index 000000000..316cb08ba --- /dev/null +++ b/internal/api/provider/azure_test.go @@ -0,0 +1,29 @@ +package provider + +import "testing" + +func TestIsAzureIssuer(t *testing.T) { + positiveExamples := []string{ + "https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0", + "https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0/", + "https://login.microsoftonline.com/common/v2.0", + } + + negativeExamples := []string{ + "http://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0", + "https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0?something=else", + "https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0/extra", + } + + for _, example := range positiveExamples { + if !IsAzureIssuer(example) { + t.Errorf("Example %q should be treated as a valid Azure issuer", example) + } + } + + for _, example := range negativeExamples { + if IsAzureIssuer(example) { + t.Errorf("Example %q should be treated as not a valid Azure issuer", example) + } + } +} diff --git a/internal/api/provider/oidc.go b/internal/api/provider/oidc.go index fe3c5c505..53baf9845 100644 --- a/internal/api/provider/oidc.go +++ b/internal/api/provider/oidc.go @@ -6,6 +6,7 @@ import ( "fmt" "strconv" "strings" + "time" "github.com/coreos/go-oidc/v3/oidc" "github.com/golang-jwt/jwt" @@ -21,6 +22,10 @@ type ParseIDTokenOptions struct { // used in tests. var OverrideVerifiers = make(map[string]func(context.Context, *oidc.Config) *oidc.IDTokenVerifier) +// OverrideClock can be used to set a custom clock function to be used when +// parsing ID tokens. Should only be used in tests. +var OverrideClock func() time.Time + func ParseIDToken(ctx context.Context, provider *oidc.Provider, config *oidc.Config, idToken string, options ParseIDTokenOptions) (*oidc.IDToken, *UserProvidedData, error) { if config == nil { config = &oidc.Config{ @@ -29,6 +34,12 @@ func ParseIDToken(ctx context.Context, provider *oidc.Provider, config *oidc.Con } } + if OverrideClock != nil { + clonedConfig := *config + clonedConfig.Now = OverrideClock + config = &clonedConfig + } + verifier := provider.VerifierContext(ctx, config) overrideVerifier, ok := OverrideVerifiers[provider.Endpoint().AuthURL] if ok && overrideVerifier != nil { @@ -50,7 +61,11 @@ func ParseIDToken(ctx context.Context, provider *oidc.Provider, config *oidc.Con case IssuerLinkedin: token, data, err = parseLinkedinIDToken(token) default: - token, data, err = parseGenericIDToken(token) + if IsAzureIssuer(token.Issuer) { + token, data, err = parseAzureIDToken(token) + } else { + token, data, err = parseGenericIDToken(token) + } } if err != nil { @@ -200,6 +215,117 @@ func parseLinkedinIDToken(token *oidc.IDToken) (*oidc.IDToken, *UserProvidedData return token, &data, nil } +type AzureIDTokenClaims struct { + jwt.StandardClaims + + Email string `json:"email"` + Name string `json:"name"` + PreferredUsername string `json:"preferred_username"` + XMicrosoftEmailDomainOwnerVerified any `json:"xms_edov"` +} + +func (c *AzureIDTokenClaims) IsEmailVerified() bool { + emailVerified := false + + edov := c.XMicrosoftEmailDomainOwnerVerified + + // If xms_edov is not set, and an email is present or xms_edov is true, + // only then is the email regarded as verified. + // https://learn.microsoft.com/en-us/azure/active-directory/develop/migrate-off-email-claim-authorization#using-the-xms_edov-optional-claim-to-determine-email-verification-status-and-migrate-users + if edov == nil { + // An email is provided, but xms_edov is not -- probably not + // configured, so we must assume the email is verified as Azure + // will only send out a potentially unverified email address in + // single-tenanat apps. + emailVerified = c.Email != "" + } else { + edovBool := false + + // Azure can't be trusted with how they encode the xms_edov + // claim. Sometimes it's "xms_edov": "1", sometimes "xms_edov": true. + switch v := edov.(type) { + case bool: + edovBool = v + + case string: + edovBool = v == "1" || v == "true" + + default: + edovBool = false + } + + emailVerified = c.Email != "" && edovBool + } + + return emailVerified +} + +// removeAzureClaimsFromCustomClaims contains the list of claims to be removed +// from the CustomClaims map. See: +// https://learn.microsoft.com/en-us/azure/active-directory/develop/id-token-claims-reference +var removeAzureClaimsFromCustomClaims = []string{ + "aud", + "iss", + "iat", + "nbf", + "exp", + "c_hash", + "at_hash", + "aio", + "nonce", + "rh", + "uti", + "jti", + "ver", + "sub", + "name", + "preferred_username", +} + +func parseAzureIDToken(token *oidc.IDToken) (*oidc.IDToken, *UserProvidedData, error) { + var data UserProvidedData + + var azureClaims AzureIDTokenClaims + if err := token.Claims(&azureClaims); err != nil { + return nil, nil, err + } + + data.Metadata = &Claims{ + Issuer: token.Issuer, + Subject: token.Subject, + Email: azureClaims.Email, + EmailVerified: azureClaims.IsEmailVerified(), + ProviderId: token.Subject, + PreferredUsername: azureClaims.PreferredUsername, + FullName: azureClaims.Name, + CustomClaims: make(map[string]any), + } + + if data.Metadata.Email != "" { + data.Emails = append(data.Emails, Email{ + Email: data.Metadata.Email, + Verified: data.Metadata.EmailVerified, + Primary: true, + }) + } + + if err := token.Claims(&data.Metadata.CustomClaims); err != nil { + return nil, nil, err + } + + if data.Metadata.CustomClaims != nil { + for _, claim := range removeAzureClaimsFromCustomClaims { + delete(data.Metadata.CustomClaims, claim) + } + } + + if len(data.Emails) <= 0 { + return nil, nil, fmt.Errorf("provider: Azure OIDC ID token from issuer %q must contain a valid email address", token.Issuer) + } + + return token, &data, nil +} + func parseGenericIDToken(token *oidc.IDToken) (*oidc.IDToken, *UserProvidedData, error) { var data UserProvidedData diff --git a/internal/api/provider/oidc_test.go b/internal/api/provider/oidc_test.go index a1f90a7f3..fff897b35 100644 --- a/internal/api/provider/oidc_test.go +++ b/internal/api/provider/oidc_test.go @@ -44,6 +44,29 @@ func googleIDTokenVerifier(ctx context.Context, config *oidc.Config) *oidc.IDTok ) } +func azureIDTokenVerifier(ctx context.Context, config *oidc.Config) *oidc.IDTokenVerifier { + keyBytes, err := base64.RawURLEncoding.DecodeString("1djHqyNclRpJWtHCnkP5QWvDxozCTG_ZDnkEmudpcxjnYrVL4RVIwdNCBLAStg8Dob5OUyAlHcRFMCqGTW4HA6kHgIxyfiFsYCBDMHWd2-61N1cAS6S9SdXlWXkBQgU0Qj6q_yFYTRS7J-zI_jMLRQAlpowfDFM1vSTBIci7kqynV6pPOz4jMaDQevmSscEs-jz7e8YXAiiVpN588oBQ0jzQaTTx90WjgRP23mn8mPyabj8gcR3gLwKLsBUhlp1oZj7FopGp8z8LHuueJB_q_LOUa_gAozZ0lfoJxFimXgpgEK7GNVdMRsMH3mIl0A5oYN8f29RFwbG0rNO5ZQ1YWQ") + if err != nil { + panic(err) + } + + n := big.NewInt(0) + n.SetBytes(keyBytes) + + publicKey := &rsa.PublicKey{ + N: n, + E: 65537, + } + + return oidc.NewVerifier( + IssuerAzureMicrosoft, + &oidc.StaticKeySet{ + PublicKeys: []crypto.PublicKey{publicKey}, + }, + config, + ) +} + var realIDTokens map[string]realIDToken = map[string]realIDToken{ IssuerGoogle: { AccessToken: "ya29.a0AWY7CklOn4TehiT4kA6osNP6e-pHErOY8X53T2oUe7Oqqwc3-uIJpoEgoZCUogewBuNWr-JFT2FK9s0E0oRSFtAfu0-uIDckBj5ca1pxnk0-zPkPZouqoIyl0AlIpQjIUEuyuQTYUay99kRajbHcFCR1VMbNcQaCgYKAQESARESFQG1tDrp1joUHupV5Rn8-nWDpKkmMw0165", @@ -51,11 +74,18 @@ var realIDTokens map[string]realIDToken = map[string]realIDToken{ Time: time.Unix(1686659933, 0), // 1 sec after iat Verifier: googleIDTokenVerifier, }, + IssuerAzureMicrosoft: { + AccessToken: "access-token", + Time: time.Unix(1697277774, 0), // 1 sec after iat + IDToken: "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6IlhvdVhMWVExVGlwNW9kWWFqaUN0RlZnVmFFcyJ9.eyJ2ZXIiOiIyLjAiLCJpc3MiOiJodHRwczovL2xvZ2luLm1pY3Jvc29mdG9ubGluZS5jb20vOTE4ODA0MGQtNmM2Ny00YzViLWIxMTItMzZhMzA0YjY2ZGFkL3YyLjAiLCJzdWIiOiJBQUFBQUFBQUFBQUFBQUFBQUFBQUFCWkRuRDkxOTBfc2wxcTZwenZlRHZNIiwiYXVkIjoiYTBkOGY5NzItNTRhYy00YWJmLTkxNGMtNTIyMDE0YzQwMjJhIiwiZXhwIjoxNjk3MzY0NDczLCJpYXQiOjE2OTcyNzc3NzMsIm5iZiI6MTY5NzI3Nzc3MywiZW1haWwiOiJzZGltaXRyb3Zza2lAZ21haWwuY29tIiwidGlkIjoiOTE4ODA0MGQtNmM2Ny00YzViLWIxMTItMzZhMzA0YjY2ZGFkIiwieG1zX2Vkb3YiOiIxIiwiYWlvIjoiRHBQV3lZSnRJcUl5OHpyVjROIUlIdGtFa09BMDhPS29lZ1RkYmZQUEVPYmxtYk9ESFQ0cGJVcVI1cExraENyWWZ6bUgzb3A1RzN5RGp2M0tNZ0Rad29lQ1FjKmVueldyb21iQ3BuKkR6OEpQOGMxU3pEVG1TbGp4U3U3UnVLTXNZSjRvS1lDazFBSVcqUUNUTmlMWkpUKlN3WWZQcjZBTW9IejFEZ3pBZEFkbk9uWiFHNUNFeEtQalBxcHRuVmpUZlEkJCJ9.CskICxOaeqd4SkiPdWEHJKZVdhAdgzM5SN7K7FYi0dguQH1-v6XTetDIoEsBn0GZoozXjbG2GgkFcVhhBvNA0ZrDIr4KcjfnJ5-7rwX3AtxdQ3umrHRlGu3jlmbDOtWzPWNMLLRXfR1Mm3pHEUvlzqmk3Ffh4TuAmXID-fb-Xmfuuv1k0UsZ5mlr_3ybTPVZk-Lj0bqkR1L5Zzt4HjgfpchRryJ3Y24b4dDsSjg7mgE_5JivgjhtVef5OnqYhKUF1DTy2pFysFO_eRliK6qjouYeZnQOJnWHP1MgpySAOQ3sVcwvE4P9g7V3QouxByZPv-g99N1K4GwZrtdm46gtTQ", + Verifier: azureIDTokenVerifier, + }, } func TestParseIDToken(t *testing.T) { defer func() { OverrideVerifiers = make(map[string]func(context.Context, *oidc.Config) *oidc.IDTokenVerifier) + OverrideClock = nil }() // note that this test can fail if/when the issuers rotate their @@ -81,3 +111,75 @@ func TestParseIDToken(t *testing.T) { require.Equal(t, user.Emails[0].Verified, true) } } + +func TestAzureIDTOkenClaimsIsEmailVerified(t *testing.T) { + positiveExamples := []AzureIDTokenClaims{ + { + Email: "test@example.com", + XMicrosoftEmailDomainOwnerVerified: nil, + }, + { + Email: "test@example.com", + XMicrosoftEmailDomainOwnerVerified: true, + }, + { + Email: "test@example.com", + XMicrosoftEmailDomainOwnerVerified: "1", + }, + { + Email: "test@example.com", + XMicrosoftEmailDomainOwnerVerified: "true", + }, + } + + negativeExamples := []AzureIDTokenClaims{ + { + Email: "", + XMicrosoftEmailDomainOwnerVerified: true, + }, + { + Email: "test@example.com", + XMicrosoftEmailDomainOwnerVerified: false, + }, + { + Email: "test@example.com", + XMicrosoftEmailDomainOwnerVerified: "0", + }, + { + Email: "test@example.com", + XMicrosoftEmailDomainOwnerVerified: "false", + }, + { + Email: "test@example.com", + XMicrosoftEmailDomainOwnerVerified: float32(0), + }, + { + Email: "test@example.com", + XMicrosoftEmailDomainOwnerVerified: float64(0), + }, + { + Email: "test@example.com", + XMicrosoftEmailDomainOwnerVerified: int(0), + }, + { + Email: "test@example.com", + XMicrosoftEmailDomainOwnerVerified: int32(0), + }, + { + Email: "test@example.com", + XMicrosoftEmailDomainOwnerVerified: int64(0), + }, + } + + for i, example := range positiveExamples { + if !example.IsEmailVerified() { + t.Errorf("positive example %v reports negative result", i) + } + } + + for i, example := range negativeExamples { + if example.IsEmailVerified() { + t.Errorf("negative example %v reports positive result", i) + } + } +} From 9441366645f67148d6e49c0673c12633c53140e8 Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Mon, 16 Oct 2023 12:10:16 +0200 Subject: [PATCH 2/2] apply suggestion from @J0 Co-authored-by: Joel Lee --- internal/api/provider/oidc_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/provider/oidc_test.go b/internal/api/provider/oidc_test.go index fff897b35..e088cd45f 100644 --- a/internal/api/provider/oidc_test.go +++ b/internal/api/provider/oidc_test.go @@ -112,7 +112,7 @@ func TestParseIDToken(t *testing.T) { } } -func TestAzureIDTOkenClaimsIsEmailVerified(t *testing.T) { +func TestAzureIDTokenClaimsIsEmailVerified(t *testing.T) { positiveExamples := []AzureIDTokenClaims{ { Email: "test@example.com",