From a34a3b2a98f5e9d549dcdf69b8d70c6791d2dbd8 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Tue, 7 Mar 2023 14:43:42 +0100 Subject: [PATCH] Cleanup some oidc related bits (#5751) * Remove unused code from oidc module * Use already existing Metadata type for jwks discovery ocis-pkg/oidc already provides a type for the oidc metadata. Switch to that instead of defining yet another custom type. * oidc: Add helper to get IDP metadata --- ocis-pkg/oidc/claims.go | 185 --------------------- ocis-pkg/oidc/metadata.go | 84 ++++++++++ ocis-pkg/oidc/option.go | 57 ------- services/proxy/pkg/middleware/oidc_auth.go | 33 +--- 4 files changed, 87 insertions(+), 272 deletions(-) create mode 100644 ocis-pkg/oidc/metadata.go delete mode 100644 ocis-pkg/oidc/option.go diff --git a/ocis-pkg/oidc/claims.go b/ocis-pkg/oidc/claims.go index 815281fc9..b6d4c5016 100644 --- a/ocis-pkg/oidc/claims.go +++ b/ocis-pkg/oidc/claims.go @@ -12,188 +12,3 @@ const ( OwncloudUUID = "ownclouduuid" OcisRoutingPolicy = "ocis.routing.policy" ) - -// The ProviderMetadata describes an idp. -// see https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata -type ProviderMetadata struct { - AuthorizationEndpoint string `json:"authorization_endpoint,omitempty"` - //claims_parameter_supported - ClaimsSupported []string `json:"claims_supported,omitempty"` - //grant_types_supported - IDTokenSigningAlgValuesSupported []string `json:"id_token_signing_alg_values_supported,omitempty"` - Issuer string `json:"issuer,omitempty"` - JwksURI string `json:"jwks_uri,omitempty"` - //registration_endpoint - //request_object_signing_alg_values_supported - //request_parameter_supported - //request_uri_parameter_supported - //require_request_uri_registration - //response_modes_supported - ResponseTypesSupported []string `json:"response_types_supported,omitempty"` - ScopesSupported []string `json:"scopes_supported,omitempty"` - SubjectTypesSupported []string `json:"subject_types_supported,omitempty"` - TokenEndpoint string `json:"token_endpoint,omitempty"` - //token_endpoint_auth_methods_supported - //token_endpoint_auth_signing_alg_values_supported - UserinfoEndpoint string `json:"userinfo_endpoint,omitempty"` - //userinfo_signing_alg_values_supported - //code_challenge_methods_supported - IntrospectionEndpoint string `json:"introspection_endpoint,omitempty"` - //introspection_endpoint_auth_methods_supported - //introspection_endpoint_auth_signing_alg_values_supported - RevocationEndpoint string `json:"revocation_endpoint,omitempty"` - //revocation_endpoint_auth_methods_supported - //revocation_endpoint_auth_signing_alg_values_supported - //id_token_encryption_alg_values_supported - //id_token_encryption_enc_values_supported - //userinfo_encryption_alg_values_supported - //userinfo_encryption_enc_values_supported - //request_object_encryption_alg_values_supported - //request_object_encryption_enc_values_supported - CheckSessionIframe string `json:"check_session_iframe,omitempty"` - EndSessionEndpoint string `json:"end_session_endpoint,omitempty"` - //claim_types_supported -} - -// StandardClaims will be stored in the context to be consumed by the oidc user manager -// They can be requested to be returned either in the UserInfo Response, per -// Section 5.3.2, or in the ID Token, per Section 2. -// see https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims -type StandardClaims struct { - // Time the End-User's information was last updated. Its value is a - // JSON number representing the number of seconds from 1970-01-01T0:0:0Z - // as measured in UTC until the date/time. - UpdatedAt int64 `json:"updated_at,omitempty"` - - // True if the End-User's e-mail address has been verified; otherwise false. - // When this Claim Value is true, this means that the OP took affirmative - // steps to ensure that this e-mail address was controlled by the End-User - // at the time the verification was performed. The means by which an e-mail - // address is verified is context-specific, and dependent upon the trust - // framework or contractual agreements within which the parties are operating. - EmailVerified bool `json:"email_verified,omitempty"` - - // True if the End-User's phone number has been verified; otherwise false. - // When this Claim Value is true, this means that the OP took affirmative - // steps to ensure that this phone number was controlled by the End-User - // at the time the verification was performed. The means by which a phone - // number is verified is context-specific, and dependent upon the trust - // framework or contractual agreements within which the parties are - // operating. When true, the phone_number Claim MUST be in E.164 format - // and any extensions MUST be represented in RFC 3966 format. - PhoneNumberVerified bool `json:"phone_number_verified,omitempty"` - - Iss string `json:"iss"` - - // Subject - Identifier for the End-User at the Issuer. - Sub string `json:"sub,omitempty"` - - // End-User's full name in displayable form including all name parts, possibly - // including titles and suffixes, ordered according to the End-User's locale - // and preferences. - Name string `json:"name,omitempty"` - - // Given name(s) or first name(s) of the End-User. Note that in some cultures, - // people can have multiple given names; all can be present, with the names - // being separated by space characters. - GivenName string `json:"given_name,omitempty"` - - // Surname(s) or last name(s) of the End-User. Note that in some cultures, - // people can have multiple family names or no family name; all can be present, - // with the names being separated by space characters. - FamilyName string `json:"family_name,omitempty"` - - // Middle name(s) of the End-User. Note that in some cultures, people can have - // multiple middle names; all can be present, with the names being separated by - // space characters. Also note that in some cultures, middle names are not used. - MiddleName string `json:"middle_name,omitempty"` - - // Casual name of the End-User that may or may not be the same as the given_name. - // For instance, a nickname value of Mike might be returned alongside a given_name - // value of Michael. - Nickname string `json:"nickname,omitempty"` - - // Shorthand name by which the End-User wishes to be referred to at the RP, such - // as janedoe or j.doe. This value MAY be any valid JSON string including special - // characters such as @, /, or whitespace. The RP MUST NOT rely upon this value - // being unique, as discussed in Section 5.7. - PreferredUsername string `json:"preferred_username,omitempty"` - - // URL of the End-User's profile page. The contents of this Web page SHOULD be - // about the End-User. - Profile string `json:"profile,omitempty"` - - // URL of the End-User's profile picture. This URL MUST refer to an image file - // (for example, a PNG, JPEG, or GIF image file), rather than to a Web page - // containing an image. Note that this URL SHOULD specifically reference a - // profile photo of the End-User suitable for displaying when describing the - // End-User, rather than an arbitrary photo taken by the End-User. - Picture string `json:"picture,omitempty"` - - // URL of the End-User's Web page or blog. This Web page SHOULD contain - // information published by the End-User or an organization that the End-User - // is affiliated with. - Website string `json:"website,omitempty"` - - // End-User's preferred e-mail address. Its value MUST conform to the RFC 5322 - // addr-spec syntax. The RP MUST NOT rely upon this value being unique, as - // discussed in Section 5.7. - Email string `json:"email,omitempty"` - - // End-User's gender. Values defined by this specification are female and male. - // Other values MAY be used when neither of the defined values are applicable. - Gender string `json:"gender,omitempty"` - - // End-User's birthday, represented as an ISO 8601:2004 YYYY-MM-DD format. - // The year MAY be 0000, indicating that it is omitted. To represent only the - // year, YYYY format is allowed. Note that depending on the underlying - // platform's date related function, providing just year can result in - // varying month and day, so the implementers need to take this factor into - // account to correctly process the dates. - Birthdate string `json:"birthdate,omitempty"` - - // String from zoneinfo time zone database representing the End-User's time - // zone. For example, Europe/Paris or America/Los_Angeles. - Zoneinfo string `json:"zoneinfo,omitempty"` - - // End-User's locale, represented as a BCP47 [RFC5646] language tag. - // This is typically an ISO 639-1 Alpha-2 [ISO639‑1] language code in - // lowercase and an ISO 3166-1 Alpha-2 [ISO3166‑1] country code in - // uppercase, separated by a dash. For example, en-US or fr-CA. As a - // compatibility note, some implementations have used an underscore as - // the separator rather than a dash, for example, en_US; Relying Parties - // MAY choose to accept this locale syntax as well. - Locale string `json:"locale,omitempty"` - - // End-User's preferred telephone number. E.164 [E.164] is RECOMMENDED - // as the format of this Claim, for example, +1 (425) 555-1212 or - // +56 (2) 687 2400. If the phone number contains an extension, it is - // RECOMMENDED that the extension be represented using the RFC 3966 - // extension syntax, for example, +1 (604) 555-1234;ext=5678. - PhoneNumber string `json:"phone_number,omitempty"` - - // TODO Name is the correct one, does kopano use display name? -> double check and report bug - DisplayName string `json:"display_name,omitempty"` - - Groups []string `json:"groups,omitempty"` - - // End-User's preferred postal address. The value of the address member - // is a JSON [RFC4627] structure containing some or all of the members - // defined in Section 5.1.1. - // TODO add address claim https://openid.net/specs/openid-connect-core-1_0.html#AddressClaim - Address map[string]interface{} `json:"address,omitempty"` - KCIdentity map[string]string `json:"kc.identity,omitempty"` - - // To integrate with an existing LDAP server the IdP can send the numeric user and group id: - - // UIDNumber is a unique numerical id that will be used when setting acls on a storage that integrates with the OS/LDAP - UIDNumber string `json:"uidnumber,omitempty"` - // GIDNumber is a unique numerical id that will be used when setting acls on a storage that integrates with the OS/LDAP - GIDNumber string `json:"gidnumber,omitempty"` - - // OcisID is a unique, persistent, non reassignable user id - OcisID string `json:"ownclouduuid,omitempty"` - - // OcisRoutingPolicy is used to specify the routing policy to use for the ocis proxy - OcisRoutingPolicy string `json:"ocis.routing.policy,omitempty"` -} diff --git a/ocis-pkg/oidc/metadata.go b/ocis-pkg/oidc/metadata.go new file mode 100644 index 000000000..bcbecaf82 --- /dev/null +++ b/ocis-pkg/oidc/metadata.go @@ -0,0 +1,84 @@ +package oidc + +import ( + "encoding/json" + "io" + "net/http" + "strings" + + "github.com/owncloud/ocis/v2/ocis-pkg/log" +) + +const wellknownPath = "/.well-known/openid-configuration" + +// The ProviderMetadata describes an idp. +// see https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata +type ProviderMetadata struct { + AuthorizationEndpoint string `json:"authorization_endpoint,omitempty"` + //claims_parameter_supported + ClaimsSupported []string `json:"claims_supported,omitempty"` + //grant_types_supported + IDTokenSigningAlgValuesSupported []string `json:"id_token_signing_alg_values_supported,omitempty"` + Issuer string `json:"issuer,omitempty"` + JwksURI string `json:"jwks_uri,omitempty"` + //registration_endpoint + //request_object_signing_alg_values_supported + //request_parameter_supported + //request_uri_parameter_supported + //require_request_uri_registration + //response_modes_supported + ResponseTypesSupported []string `json:"response_types_supported,omitempty"` + ScopesSupported []string `json:"scopes_supported,omitempty"` + SubjectTypesSupported []string `json:"subject_types_supported,omitempty"` + TokenEndpoint string `json:"token_endpoint,omitempty"` + //token_endpoint_auth_methods_supported + //token_endpoint_auth_signing_alg_values_supported + UserinfoEndpoint string `json:"userinfo_endpoint,omitempty"` + //userinfo_signing_alg_values_supported + //code_challenge_methods_supported + IntrospectionEndpoint string `json:"introspection_endpoint,omitempty"` + //introspection_endpoint_auth_methods_supported + //introspection_endpoint_auth_signing_alg_values_supported + RevocationEndpoint string `json:"revocation_endpoint,omitempty"` + //revocation_endpoint_auth_methods_supported + //revocation_endpoint_auth_signing_alg_values_supported + //id_token_encryption_alg_values_supported + //id_token_encryption_enc_values_supported + //userinfo_encryption_alg_values_supported + //userinfo_encryption_enc_values_supported + //request_object_encryption_alg_values_supported + //request_object_encryption_enc_values_supported + CheckSessionIframe string `json:"check_session_iframe,omitempty"` + EndSessionEndpoint string `json:"end_session_endpoint,omitempty"` + //claim_types_supported +} + +func GetIDPMetadata(logger log.Logger, client *http.Client, idpURI string) (ProviderMetadata, error) { + wellknownURI := strings.TrimSuffix(idpURI, "/") + wellknownPath + + resp, err := client.Get(wellknownURI) + if err != nil { + logger.Error().Err(err).Msg("Failed to set request for .well-known/openid-configuration") + return ProviderMetadata{}, err + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + logger.Error().Err(err).Msg("unable to read discovery response body") + return ProviderMetadata{}, err + } + + if resp.StatusCode != http.StatusOK { + logger.Error().Str("status", resp.Status).Str("body", string(body)).Msg("error requesting openid-configuration") + return ProviderMetadata{}, err + } + + var oidcMetadata ProviderMetadata + err = json.Unmarshal(body, &oidcMetadata) + if err != nil { + logger.Error().Err(err).Msg("failed to decode provider openid-configuration") + return ProviderMetadata{}, err + } + return oidcMetadata, nil +} diff --git a/ocis-pkg/oidc/option.go b/ocis-pkg/oidc/option.go deleted file mode 100644 index 19fe5913a..000000000 --- a/ocis-pkg/oidc/option.go +++ /dev/null @@ -1,57 +0,0 @@ -package oidc - -import ( - "github.com/owncloud/ocis/v2/ocis-pkg/log" -) - -// Option defines a single option function. -type Option func(o *Options) - -// Options defines the available options for this package. -type Options struct { - // Logger to use for logging, must be set - Logger log.Logger - // Endpoint is the OpenID Connect provider URL - Endpoint string - // Realm to use in the WWW-Authenticate header, defaults to Endpoint - Realm string - // SigningAlgs to use when verifying jwt signatures, defaults to "RS256" & "PS256" - SigningAlgs []string - // Insecure can be used to disable http certificate checks - Insecure bool -} - -// Logger provides a function to set the logger option. -func Logger(l log.Logger) Option { - return func(o *Options) { - o.Logger = l - } -} - -// Endpoint provides a function to set the endpoint option. -func Endpoint(e string) Option { - return func(o *Options) { - o.Endpoint = e - } -} - -// Realm provides a function to set the realm option. -func Realm(r string) Option { - return func(o *Options) { - o.Realm = r - } -} - -// SigningAlgs provides a function to set the signing algorithms option. -func SigningAlgs(sa []string) Option { - return func(o *Options) { - o.SigningAlgs = sa - } -} - -// Insecure provides a function to set the insecure option. -func Insecure(i bool) Option { - return func(o *Options) { - o.Insecure = i - } -} diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index 32f757113..7d4f6322b 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/services/proxy/pkg/middleware/oidc_auth.go @@ -2,8 +2,6 @@ package middleware import ( "context" - "encoding/json" - "io" "net/http" "strings" "sync" @@ -164,41 +162,16 @@ func (m OIDCAuthenticator) shouldServe(req *http.Request) bool { return strings.HasPrefix(header, _bearerPrefix) } -type jwksJSON struct { - JWKSURL string `json:"jwks_uri"` -} - func (m *OIDCAuthenticator) getKeyfunc() *keyfunc.JWKS { m.jwksLock.Lock() defer m.jwksLock.Unlock() if m.JWKS == nil { - wellKnown := strings.TrimSuffix(m.OIDCIss, "/") + "/.well-known/openid-configuration" - - resp, err := m.HTTPClient.Get(wellKnown) - if err != nil { - m.Logger.Error().Err(err).Msg("Failed to set request for .well-known/openid-configuration") - return nil - } - defer resp.Body.Close() - - body, err := io.ReadAll(resp.Body) - if err != nil { - m.Logger.Error().Err(err).Msg("unable to read discovery response body") - return nil - } - - if resp.StatusCode != http.StatusOK { - m.Logger.Error().Str("status", resp.Status).Str("body", string(body)).Msg("error requesting openid-configuration") - return nil - } - - var j jwksJSON - err = json.Unmarshal(body, &j) + oidcMetadata, err := oidc.GetIDPMetadata(m.Logger, m.HTTPClient, m.OIDCIss) if err != nil { m.Logger.Error().Err(err).Msg("failed to decode provider openid-configuration") return nil } - m.Logger.Debug().Str("jwks", j.JWKSURL).Msg("discovered jwks endpoint") + m.Logger.Debug().Str("jwks", oidcMetadata.JwksURI).Msg("discovered jwks endpoint") options := keyfunc.Options{ Client: m.HTTPClient, RefreshErrorHandler: func(err error) { @@ -209,7 +182,7 @@ func (m *OIDCAuthenticator) getKeyfunc() *keyfunc.JWKS { RefreshTimeout: time.Second * time.Duration(m.JWKSOptions.RefreshTimeout), RefreshUnknownKID: m.JWKSOptions.RefreshUnknownKID, } - m.JWKS, err = keyfunc.Get(j.JWKSURL, options) + m.JWKS, err = keyfunc.Get(oidcMetadata.JwksURI, options) if err != nil { m.JWKS = nil m.Logger.Error().Err(err).Msg("Failed to create JWKS from resource at the given URL.")