diff --git a/cmd/setup/50.go b/cmd/setup/50.go new file mode 100644 index 0000000000..fea69f79ce --- /dev/null +++ b/cmd/setup/50.go @@ -0,0 +1,27 @@ +package setup + +import ( + "context" + _ "embed" + + "github.com/zitadel/zitadel/internal/database" + "github.com/zitadel/zitadel/internal/eventstore" +) + +var ( + //go:embed 50.sql + addUsePKCE string +) + +type IDPTemplate6UsePKCE struct { + dbClient *database.DB +} + +func (mig *IDPTemplate6UsePKCE) Execute(ctx context.Context, _ eventstore.Event) error { + _, err := mig.dbClient.ExecContext(ctx, addUsePKCE) + return err +} + +func (mig *IDPTemplate6UsePKCE) String() string { + return "50_idp_templates6_add_use_pkce" +} diff --git a/cmd/setup/50.sql b/cmd/setup/50.sql new file mode 100644 index 0000000000..4ff0fd7042 --- /dev/null +++ b/cmd/setup/50.sql @@ -0,0 +1,2 @@ +ALTER TABLE IF EXISTS projections.idp_templates6_oauth2 ADD COLUMN IF NOT EXISTS use_pkce BOOLEAN; +ALTER TABLE IF EXISTS projections.idp_templates6_oidc ADD COLUMN IF NOT EXISTS use_pkce BOOLEAN; \ No newline at end of file diff --git a/cmd/setup/config.go b/cmd/setup/config.go index 0153f7227f..6706d219e6 100644 --- a/cmd/setup/config.go +++ b/cmd/setup/config.go @@ -138,6 +138,7 @@ type Steps struct { s47FillMembershipFields *FillMembershipFields s48Apps7SAMLConfigsLoginVersion *Apps7SAMLConfigsLoginVersion s49InitPermittedOrgsFunction *InitPermittedOrgsFunction + s50IDPTemplate6UsePKCE *IDPTemplate6UsePKCE } func MustNewSteps(v *viper.Viper) *Steps { diff --git a/cmd/setup/setup.go b/cmd/setup/setup.go index 74b16355f3..a4bfc42403 100644 --- a/cmd/setup/setup.go +++ b/cmd/setup/setup.go @@ -175,6 +175,7 @@ func Setup(ctx context.Context, config *Config, steps *Steps, masterKey string) steps.s47FillMembershipFields = &FillMembershipFields{eventstore: eventstoreClient} steps.s48Apps7SAMLConfigsLoginVersion = &Apps7SAMLConfigsLoginVersion{dbClient: dbClient} steps.s49InitPermittedOrgsFunction = &InitPermittedOrgsFunction{eventstoreClient: dbClient} + steps.s50IDPTemplate6UsePKCE = &IDPTemplate6UsePKCE{dbClient: dbClient} err = projection.Create(ctx, dbClient, eventstoreClient, config.Projections, nil, nil, nil) logging.OnError(err).Fatal("unable to start projections") @@ -240,6 +241,7 @@ func Setup(ctx context.Context, config *Config, steps *Steps, masterKey string) steps.s46InitPermissionFunctions, steps.s47FillMembershipFields, steps.s49InitPermittedOrgsFunction, + steps.s50IDPTemplate6UsePKCE, } { mustExecuteMigration(ctx, eventstoreClient, step, "migration failed") } diff --git a/console/src/app/modules/providers/provider-oauth/provider-oauth.component.html b/console/src/app/modules/providers/provider-oauth/provider-oauth.component.html index b8f43a856a..8c6412cd91 100644 --- a/console/src/app/modules/providers/provider-oauth/provider-oauth.component.html +++ b/console/src/app/modules/providers/provider-oauth/provider-oauth.component.html @@ -110,6 +110,15 @@ +
+ +
+

{{ 'IDP.USEPKCE_DESC' | translate }}

+ {{ 'IDP.USEPKCE' | translate }} +
+ +
+ -
+

{{ 'IDP.ISIDTOKENMAPPING_DESC' | translate }}

{{ 'IDP.ISIDTOKENMAPPING' | translate }}
+ + +
+

{{ 'IDP.USEPKCE_DESC' | translate }}

+ {{ 'IDP.USEPKCE' | translate }} +
+
+ { @@ -165,6 +166,7 @@ export class ProviderOIDCComponent { req.setScopesList(this.scopesList?.value); req.setProviderOptions(this.options); req.setIsIdTokenMapping(this.isIdTokenMapping?.value); + req.setUsePkce(this.usePkce?.value); this.loading = true; this.service @@ -193,6 +195,7 @@ export class ProviderOIDCComponent { req.setScopesList(this.scopesList?.value); req.setProviderOptions(this.options); req.setIsIdTokenMapping(this.isIdTokenMapping?.value); + req.setUsePkce(this.usePkce?.value); this.loading = true; this.service @@ -261,4 +264,8 @@ export class ProviderOIDCComponent { public get isIdTokenMapping(): AbstractControl | null { return this.form.get('isIdTokenMapping'); } + + public get usePkce(): AbstractControl | null { + return this.form.get('usePkce'); + } } diff --git a/console/src/assets/i18n/bg.json b/console/src/assets/i18n/bg.json index 9571f75db3..90f6a821bd 100644 --- a/console/src/assets/i18n/bg.json +++ b/console/src/assets/i18n/bg.json @@ -2200,7 +2200,9 @@ "REMOVED": "Премахнато успешно." }, "ISIDTOKENMAPPING": "Съответствие от ID токен", - "ISIDTOKENMAPPING_DESC": "Ако е избрано, информацията на доставчика се съответства от ID токена, а не от userinfo крайната точка." + "ISIDTOKENMAPPING_DESC": "Ако е избрано, информацията на доставчика се съответства от ID токена, а не от userinfo крайната точка.", + "USEPKCE": "Използвайте PKCE", + "USEPKCE_DESC": "Определя дали параметрите code_challenge и code_challenge_method са включени в заявката за удостоверяване" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/cs.json b/console/src/assets/i18n/cs.json index 68558efab3..b77fcacf87 100644 --- a/console/src/assets/i18n/cs.json +++ b/console/src/assets/i18n/cs.json @@ -2213,7 +2213,9 @@ "REMOVED": "Úspěšně odebráno." }, "ISIDTOKENMAPPING": "Mapování z ID tokenu", - "ISIDTOKENMAPPING_DESC": "Pokud je vybráno, informace o poskytovateli jsou mapovány z ID tokenu, nikoli z koncového bodu userinfo." + "ISIDTOKENMAPPING_DESC": "Pokud je vybráno, informace o poskytovateli jsou mapovány z ID tokenu, nikoli z koncového bodu userinfo.", + "USEPKCE": "Použijte PKCE", + "USEPKCE_DESC": "Určuje, zda jsou v požadavku na ověření zahrnuty parametry code_challenge a code_challenge_method" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/de.json b/console/src/assets/i18n/de.json index af65e76365..309aa7fd2b 100644 --- a/console/src/assets/i18n/de.json +++ b/console/src/assets/i18n/de.json @@ -2204,7 +2204,9 @@ "REMOVED": "Erfolgreich entfernt." }, "ISIDTOKENMAPPING": "Zuordnung vom ID-Token", - "ISIDTOKENMAPPING_DESC": "Legt fest, ob für das Mapping der Provider Informationen das ID-Token verwendet werden soll, anstatt des Userinfo-Endpoints." + "ISIDTOKENMAPPING_DESC": "Legt fest, ob für das Mapping der Provider Informationen das ID-Token verwendet werden soll, anstatt des Userinfo-Endpoints.", + "USEPKCE": "Verwenden Sie PKCE", + "USEPKCE_DESC": "Bestimmt, ob die Parameter code_challenge und code_challenge_method in der Authentifizierungsanforderung enthalten sind" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/en.json b/console/src/assets/i18n/en.json index 40c215e1bf..53d4f0c898 100644 --- a/console/src/assets/i18n/en.json +++ b/console/src/assets/i18n/en.json @@ -2225,7 +2225,9 @@ "REMOVED": "Removed successfully." }, "ISIDTOKENMAPPING": "Map from the ID token", - "ISIDTOKENMAPPING_DESC": "If selected, provider information gets mapped from the ID token, not from the userinfo endpoint." + "ISIDTOKENMAPPING_DESC": "If selected, provider information gets mapped from the ID token, not from the userinfo endpoint.", + "USEPKCE": "Use PKCE", + "USEPKCE_DESC": "Determines whether the code_challenge and code_challenge_method params are included in the auth request" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/es.json b/console/src/assets/i18n/es.json index b21b001c4d..9142d8930b 100644 --- a/console/src/assets/i18n/es.json +++ b/console/src/assets/i18n/es.json @@ -2201,7 +2201,9 @@ "REMOVED": "Eliminado con éxito." }, "ISIDTOKENMAPPING": "Asignación del ID token", - "ISIDTOKENMAPPING_DESC": "Si se selecciona, la información del proveedor se asigna desde el ID token, no desde el punto final de userinfo." + "ISIDTOKENMAPPING_DESC": "Si se selecciona, la información del proveedor se asigna desde el ID token, no desde el punto final de userinfo.", + "USEPKCE": "Usa PKCE", + "USEPKCE_DESC": "Determina si los parámetros code_challenge y code_challenge_method son incluidos en la solicitud de autenticación." }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/fr.json b/console/src/assets/i18n/fr.json index 6b3cd356d1..1de197356e 100644 --- a/console/src/assets/i18n/fr.json +++ b/console/src/assets/i18n/fr.json @@ -2205,7 +2205,9 @@ "REMOVED": "Suppression réussie." }, "ISIDTOKENMAPPING": "Mappage depuis le jeton ID", - "ISIDTOKENMAPPING_DESC": "Si sélectionné, les informations du fournisseur sont mappées à partir du jeton ID, et non à partir du point d'extrémité userinfo." + "ISIDTOKENMAPPING_DESC": "Si sélectionné, les informations du fournisseur sont mappées à partir du jeton ID, et non à partir du point d'extrémité userinfo.", + "USEPKCE": "Utiliser PKCE", + "USEPKCE_DESC": "Détermine si les paramètres code_challenge et code_challenge_method sont inclus dans la demande d'authentification" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/hu.json b/console/src/assets/i18n/hu.json index dc9db9ed2f..71d7421ffb 100644 --- a/console/src/assets/i18n/hu.json +++ b/console/src/assets/i18n/hu.json @@ -2223,7 +2223,9 @@ "REMOVED": "Sikeresen eltávolítva." }, "ISIDTOKENMAPPING": "Hozzárendelés az ID token alapján", - "ISIDTOKENMAPPING_DESC": "Ha ezt választod, a szolgáltatói információkat az ID token alapján rendeljük hozzá, nem a userinfo végpontból." + "ISIDTOKENMAPPING_DESC": "Ha ezt választod, a szolgáltatói információkat az ID token alapján rendeljük hozzá, nem a userinfo végpontból.", + "USEPKCE": "PKCE használata", + "USEPKCE_DESC": "Meghatározza, hogy a code_challenge és a code_challenge_method paraméterek szerepeljenek-e a hitelesítési kérelemben" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/id.json b/console/src/assets/i18n/id.json index 50c897a752..d4b4150a13 100644 --- a/console/src/assets/i18n/id.json +++ b/console/src/assets/i18n/id.json @@ -2006,7 +2006,9 @@ "REMOVED": "Berhasil dihapus." }, "ISIDTOKENMAPPING": "Peta dari token ID", - "ISIDTOKENMAPPING_DESC": "Jika dipilih, informasi penyedia akan dipetakan dari token ID, bukan dari titik akhir info pengguna." + "ISIDTOKENMAPPING_DESC": "Jika dipilih, informasi penyedia akan dipetakan dari token ID, bukan dari titik akhir info pengguna.", + "USEPKCE": "Gunakan PKCE", + "USEPKCE_DESC": "Menentukan apakah parameter code_challenge dan code_challenge_method disertakan dalam permintaan autentikasi" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/it.json b/console/src/assets/i18n/it.json index 0f8c324d3b..1b944cc517 100644 --- a/console/src/assets/i18n/it.json +++ b/console/src/assets/i18n/it.json @@ -2205,7 +2205,9 @@ "REMOVED": "Rimosso con successo." }, "ISIDTOKENMAPPING": "Mappatura dal token ID", - "ISIDTOKENMAPPING_DESC": "Se selezionato, le informazioni del provider vengono mappate dal token ID, non dal punto finale userinfo." + "ISIDTOKENMAPPING_DESC": "Se selezionato, le informazioni del provider vengono mappate dal token ID, non dal punto finale userinfo.", + "USEPKCE": "Usa PKCE", + "USEPKCE_DESC": "Determina se i parametri code_challenge e code_challenge_method sono inclusi nella richiesta di autenticazione" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/ja.json b/console/src/assets/i18n/ja.json index effc368463..8f4b054fb9 100644 --- a/console/src/assets/i18n/ja.json +++ b/console/src/assets/i18n/ja.json @@ -2225,7 +2225,9 @@ "REMOVED": "正常に削除されました。" }, "ISIDTOKENMAPPING": "IDトークンからのマッピング", - "ISIDTOKENMAPPING_DESC": "選択された場合、プロバイダ情報はIDトークンからマッピングされ、userinfoエンドポイントからではありません。" + "ISIDTOKENMAPPING_DESC": "選択された場合、プロバイダ情報はIDトークンからマッピングされ、userinfoエンドポイントからではありません。", + "USEPKCE": "PKCEを使用する", + "USEPKCE_DESC": "code_challenge パラメータと code_challenge_method パラメータが認証リクエストに含まれるかどうかを決定します。" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/ko.json b/console/src/assets/i18n/ko.json index 14aec2246e..9c077d74d7 100644 --- a/console/src/assets/i18n/ko.json +++ b/console/src/assets/i18n/ko.json @@ -2225,7 +2225,9 @@ "REMOVED": "성공적으로 제거되었습니다." }, "ISIDTOKENMAPPING": "ID 토큰에서 매핑", - "ISIDTOKENMAPPING_DESC": "선택 시, 사용자 정보 엔드포인트가 아닌 ID 토큰에서 제공자 정보를 매핑합니다." + "ISIDTOKENMAPPING_DESC": "선택 시, 사용자 정보 엔드포인트가 아닌 ID 토큰에서 제공자 정보를 매핑합니다.", + "USEPKCE": "PKCE 사용", + "USEPKCE_DESC": "code_challenge 및 code_challenge_method 매개변수가 인증 요청에 포함되는지 여부를 결정합니다" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/mk.json b/console/src/assets/i18n/mk.json index 51c645e233..9d8e4bd95d 100644 --- a/console/src/assets/i18n/mk.json +++ b/console/src/assets/i18n/mk.json @@ -2201,7 +2201,9 @@ "REMOVED": "Успешно отстрането." }, "ISIDTOKENMAPPING": "Совпаѓање од ID токен", - "ISIDTOKENMAPPING_DESC": "Ако е избрано, информациите од провајдерот се мапираат од ID токенот, а не од userinfo крајната точка." + "ISIDTOKENMAPPING_DESC": "Ако е избрано, информациите од провајдерот се мапираат од ID токенот, а не од userinfo крајната точка.", + "USEPKCE": "Користете PKCE", + "USEPKCE_DESC": "Определува дали параметрите code_challenge и code_challenge_method се вклучени во барањето за авторизација" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/nl.json b/console/src/assets/i18n/nl.json index 0914858c85..306a5f44a2 100644 --- a/console/src/assets/i18n/nl.json +++ b/console/src/assets/i18n/nl.json @@ -2220,7 +2220,9 @@ "REMOVED": "Succesvol verwijderd." }, "ISIDTOKENMAPPING": "Kaart van de ID token", - "ISIDTOKENMAPPING_DESC": "Als geselecteerd, wordt provider informatie in kaart gebracht van de ID token, niet van de userinfo eindpunt." + "ISIDTOKENMAPPING_DESC": "Als geselecteerd, wordt provider informatie in kaart gebracht van de ID token, niet van de userinfo eindpunt.", + "USEPKCE": "Gebruik PKCE", + "USEPKCE_DESC": "Bepaalt of de parameters code_challenge en code_challenge_method zijn opgenomen in het verificatieverzoek" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/pl.json b/console/src/assets/i18n/pl.json index a8987142d7..59bc20ff35 100644 --- a/console/src/assets/i18n/pl.json +++ b/console/src/assets/i18n/pl.json @@ -2204,7 +2204,9 @@ "REMOVED": "Usunięto pomyślnie." }, "ISIDTOKENMAPPING": "Mapowanie z tokena ID", - "ISIDTOKENMAPPING_DESC": "Jeśli wybrane, informacje dostawcy są mapowane z tokena ID, a nie z punktu końcowego userinfo." + "ISIDTOKENMAPPING_DESC": "Jeśli wybrane, informacje dostawcy są mapowane z tokena ID, a nie z punktu końcowego userinfo.", + "USEPKCE": "Skorzystaj z PKCE", + "USEPKCE_DESC": "Określa, czy parametry code_challenge i code_challenge_method są uwzględnione w żądaniu uwierzytelnienia" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/pt.json b/console/src/assets/i18n/pt.json index cb1624ba3a..d901089823 100644 --- a/console/src/assets/i18n/pt.json +++ b/console/src/assets/i18n/pt.json @@ -2200,7 +2200,9 @@ "REMOVED": "Removido com sucesso." }, "ISIDTOKENMAPPING": "Mapeamento do token ID", - "ISIDTOKENMAPPING_DESC": "Se selecionado, as informações do provedor são mapeadas a partir do token ID, e não do ponto final userinfo." + "ISIDTOKENMAPPING_DESC": "Se selecionado, as informações do provedor são mapeadas a partir do token ID, e não do ponto final userinfo.", + "USEPKCE": "Usar PKCE", + "USEPKCE_DESC": "Determina se os parâmetros code_challenge e code_challenge_method estão incluídos na solicitação de autenticação" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/ru.json b/console/src/assets/i18n/ru.json index 123b00122e..38983410b6 100644 --- a/console/src/assets/i18n/ru.json +++ b/console/src/assets/i18n/ru.json @@ -2316,7 +2316,9 @@ "REMOVED": "Удалено успешно." }, "ISIDTOKENMAPPING": "Карта из ID-токена", - "ISIDTOKENMAPPING_DESC": "Если этот флажок установлен, информация о поставщике сопоставляется с маркером идентификатора, а не с конечной точкой информации о пользователе." + "ISIDTOKENMAPPING_DESC": "Если этот флажок установлен, информация о поставщике сопоставляется с маркером идентификатора, а не с конечной точкой информации о пользователе.", + "USEPKCE": "Используйте ПКСЕ", + "USEPKCE_DESC": "Определяет, включены ли параметры code_challenge и code_challenge_method в запрос аутентификации." }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/sv.json b/console/src/assets/i18n/sv.json index aeb3e31074..84ac42ac0b 100644 --- a/console/src/assets/i18n/sv.json +++ b/console/src/assets/i18n/sv.json @@ -2229,7 +2229,9 @@ "REMOVED": "Borttagen framgångsrikt." }, "ISIDTOKENMAPPING": "Mappa från ID-token", - "ISIDTOKENMAPPING_DESC": "Om valt, mappas leverantörsinformation från ID-token, inte från användarinfo-slutpunkten." + "ISIDTOKENMAPPING_DESC": "Om valt, mappas leverantörsinformation från ID-token, inte från användarinfo-slutpunkten.", + "USEPKCE": "Använd PKCE", + "USEPKCE_DESC": "Avgör om parametrarna code_challenge och code_challenge_method ingår i autentiseringsbegäran" }, "MFA": { "LIST": { diff --git a/console/src/assets/i18n/zh.json b/console/src/assets/i18n/zh.json index fb2eb1a080..7dbc03b2fd 100644 --- a/console/src/assets/i18n/zh.json +++ b/console/src/assets/i18n/zh.json @@ -2204,7 +2204,9 @@ "REMOVED": "成功删除。" }, "ISIDTOKENMAPPING": "从ID令牌映射", - "ISIDTOKENMAPPING_DESC": "如果选中，提供商信息将从ID令牌映射，而不是从userinfo端点。" + "ISIDTOKENMAPPING_DESC": "如果选中，提供商信息将从ID令牌映射，而不是从userinfo端点。", + "USEPKCE": "使用PKCE", + "USEPKCE_DESC": "确定 auth 请求中是否包含 code_challenge 和 code_challenge_method 参数" }, "MFA": { "LIST": { diff --git a/docs/docs/guides/integrate/identity-providers/keycloak.mdx b/docs/docs/guides/integrate/identity-providers/keycloak.mdx index 8eb619d8b2..1864884118 100644 --- a/docs/docs/guides/integrate/identity-providers/keycloak.mdx +++ b/docs/docs/guides/integrate/identity-providers/keycloak.mdx @@ -50,6 +50,9 @@ A useful default will be filled if you don't change anything. This information will be taken to create/update the user within ZITADEL. ZITADEL ensures that at least the `openid`-scope is always sent. +**Use PKCE**: If enabled, the provider will use Proof Key for Code Exchange (PKCE) to secure the authorization code flow +in addition to the client secret. + ![Keycloak Provider](/img/guides/zitadel_keycloak_create_provider.png) diff --git a/docs/docs/guides/integrate/identity-providers/okta-oidc.mdx b/docs/docs/guides/integrate/identity-providers/okta-oidc.mdx index f96043d941..0c2a46000a 100644 --- a/docs/docs/guides/integrate/identity-providers/okta-oidc.mdx +++ b/docs/docs/guides/integrate/identity-providers/okta-oidc.mdx @@ -49,6 +49,9 @@ A useful default will be filled if you don't change anything. This information will be taken to create/update the user within ZITADEL. ZITADEL ensures that at least the `openid`-scope is always sent. +**Use PKCE**: If enabled, the provider will use Proof Key for Code Exchange (PKCE) to secure the authorization code flow +in addition to the client secret. + ### Activate IdP diff --git a/internal/api/grpc/admin/idp_converter.go b/internal/api/grpc/admin/idp_converter.go index e181542384..3084031a73 100644 --- a/internal/api/grpc/admin/idp_converter.go +++ b/internal/api/grpc/admin/idp_converter.go @@ -215,6 +215,7 @@ func addGenericOAuthProviderToCommand(req *admin_pb.AddGenericOAuthProviderReque UserEndpoint: req.UserEndpoint, Scopes: req.Scopes, IDAttribute: req.IdAttribute, + UsePKCE: req.UsePkce, IDPOptions: idp_grpc.OptionsToCommand(req.ProviderOptions), } } @@ -229,6 +230,7 @@ func updateGenericOAuthProviderToCommand(req *admin_pb.UpdateGenericOAuthProvide UserEndpoint: req.UserEndpoint, Scopes: req.Scopes, IDAttribute: req.IdAttribute, + UsePKCE: req.UsePkce, IDPOptions: idp_grpc.OptionsToCommand(req.ProviderOptions), } } @@ -241,6 +243,7 @@ func addGenericOIDCProviderToCommand(req *admin_pb.AddGenericOIDCProviderRequest ClientSecret: req.ClientSecret, Scopes: req.Scopes, IsIDTokenMapping: req.IsIdTokenMapping, + UsePKCE: req.UsePkce, IDPOptions: idp_grpc.OptionsToCommand(req.ProviderOptions), } } @@ -253,6 +256,7 @@ func updateGenericOIDCProviderToCommand(req *admin_pb.UpdateGenericOIDCProviderR ClientSecret: req.ClientSecret, Scopes: req.Scopes, IsIDTokenMapping: req.IsIdTokenMapping, + UsePKCE: req.UsePkce, IDPOptions: idp_grpc.OptionsToCommand(req.ProviderOptions), } } diff --git a/internal/api/grpc/idp/converter.go b/internal/api/grpc/idp/converter.go index 8edc585fe8..6269f122c0 100644 --- a/internal/api/grpc/idp/converter.go +++ b/internal/api/grpc/idp/converter.go @@ -504,6 +504,7 @@ func oauthConfigToPb(providerConfig *idp_pb.ProviderConfig, template *query.OAut UserEndpoint: template.UserEndpoint, Scopes: template.Scopes, IdAttribute: template.IDAttribute, + UsePkce: template.UsePKCE, }, } } @@ -515,6 +516,7 @@ func oidcConfigToPb(providerConfig *idp_pb.ProviderConfig, template *query.OIDCI Issuer: template.Issuer, Scopes: template.Scopes, IsIdTokenMapping: template.IsIDTokenMapping, + UsePkce: template.UsePKCE, }, } } diff --git a/internal/api/grpc/management/idp_converter.go b/internal/api/grpc/management/idp_converter.go index b2fcb7652a..4f68c5f919 100644 --- a/internal/api/grpc/management/idp_converter.go +++ b/internal/api/grpc/management/idp_converter.go @@ -209,6 +209,7 @@ func addGenericOAuthProviderToCommand(req *mgmt_pb.AddGenericOAuthProviderReques Scopes: req.Scopes, IDAttribute: req.IdAttribute, IDPOptions: idp_grpc.OptionsToCommand(req.ProviderOptions), + UsePKCE: req.UsePkce, } } @@ -223,6 +224,7 @@ func updateGenericOAuthProviderToCommand(req *mgmt_pb.UpdateGenericOAuthProvider Scopes: req.Scopes, IDAttribute: req.IdAttribute, IDPOptions: idp_grpc.OptionsToCommand(req.ProviderOptions), + UsePKCE: req.UsePkce, } } @@ -234,6 +236,7 @@ func addGenericOIDCProviderToCommand(req *mgmt_pb.AddGenericOIDCProviderRequest) ClientSecret: req.ClientSecret, Scopes: req.Scopes, IsIDTokenMapping: req.IsIdTokenMapping, + UsePKCE: req.UsePkce, IDPOptions: idp_grpc.OptionsToCommand(req.ProviderOptions), } } @@ -246,6 +249,7 @@ func updateGenericOIDCProviderToCommand(req *mgmt_pb.UpdateGenericOIDCProviderRe ClientSecret: req.ClientSecret, Scopes: req.Scopes, IsIDTokenMapping: req.IsIdTokenMapping, + UsePKCE: req.UsePkce, IDPOptions: idp_grpc.OptionsToCommand(req.ProviderOptions), } } diff --git a/internal/api/grpc/user/v2/user.go b/internal/api/grpc/user/v2/user.go index 9a092afacf..a743206cf0 100644 --- a/internal/api/grpc/user/v2/user.go +++ b/internal/api/grpc/user/v2/user.go @@ -368,31 +368,31 @@ func (s *Server) StartIdentityProviderIntent(ctx context.Context, req *user.Star } func (s *Server) startIDPIntent(ctx context.Context, idpID string, urls *user.RedirectURLs) (*user.StartIdentityProviderIntentResponse, error) { - intentWriteModel, details, err := s.command.CreateIntent(ctx, idpID, urls.GetSuccessUrl(), urls.GetFailureUrl(), authz.GetInstance(ctx).InstanceID()) + state, session, err := s.command.AuthFromProvider(ctx, idpID, s.idpCallback(ctx), s.samlRootURL(ctx, idpID)) if err != nil { return nil, err } - content, redirect, err := s.command.AuthFromProvider(ctx, idpID, intentWriteModel.AggregateID, s.idpCallback(ctx), s.samlRootURL(ctx, idpID)) + _, details, err := s.command.CreateIntent(ctx, state, idpID, urls.GetSuccessUrl(), urls.GetFailureUrl(), authz.GetInstance(ctx).InstanceID(), session.PersistentParameters()) if err != nil { return nil, err } + content, redirect := session.GetAuth(ctx) if redirect { return &user.StartIdentityProviderIntentResponse{ Details: object.DomainToDetailsPb(details), NextStep: &user.StartIdentityProviderIntentResponse_AuthUrl{AuthUrl: content}, }, nil - } else { - return &user.StartIdentityProviderIntentResponse{ - Details: object.DomainToDetailsPb(details), - NextStep: &user.StartIdentityProviderIntentResponse_PostForm{ - PostForm: []byte(content), - }, - }, nil } + return &user.StartIdentityProviderIntentResponse{ + Details: object.DomainToDetailsPb(details), + NextStep: &user.StartIdentityProviderIntentResponse_PostForm{ + PostForm: []byte(content), + }, + }, nil } func (s *Server) startLDAPIntent(ctx context.Context, idpID string, ldapCredentials *user.LDAPCredentials) (*user.StartIdentityProviderIntentResponse, error) { - intentWriteModel, details, err := s.command.CreateIntent(ctx, idpID, "", "", authz.GetInstance(ctx).InstanceID()) + intentWriteModel, details, err := s.command.CreateIntent(ctx, "", idpID, "", "", authz.GetInstance(ctx).InstanceID(), nil) if err != nil { return nil, err } diff --git a/internal/api/grpc/user/v2beta/user.go b/internal/api/grpc/user/v2beta/user.go index 52da057906..cf6dfa6304 100644 --- a/internal/api/grpc/user/v2beta/user.go +++ b/internal/api/grpc/user/v2beta/user.go @@ -371,31 +371,31 @@ func (s *Server) StartIdentityProviderIntent(ctx context.Context, req *user.Star } func (s *Server) startIDPIntent(ctx context.Context, idpID string, urls *user.RedirectURLs) (*user.StartIdentityProviderIntentResponse, error) { - intentWriteModel, details, err := s.command.CreateIntent(ctx, idpID, urls.GetSuccessUrl(), urls.GetFailureUrl(), authz.GetInstance(ctx).InstanceID()) + state, session, err := s.command.AuthFromProvider(ctx, idpID, s.idpCallback(ctx), s.samlRootURL(ctx, idpID)) if err != nil { return nil, err } - content, redirect, err := s.command.AuthFromProvider(ctx, idpID, intentWriteModel.AggregateID, s.idpCallback(ctx), s.samlRootURL(ctx, idpID)) + _, details, err := s.command.CreateIntent(ctx, state, idpID, urls.GetSuccessUrl(), urls.GetFailureUrl(), authz.GetInstance(ctx).InstanceID(), session.PersistentParameters()) if err != nil { return nil, err } + content, redirect := session.GetAuth(ctx) if redirect { return &user.StartIdentityProviderIntentResponse{ Details: object.DomainToDetailsPb(details), NextStep: &user.StartIdentityProviderIntentResponse_AuthUrl{AuthUrl: content}, }, nil - } else { - return &user.StartIdentityProviderIntentResponse{ - Details: object.DomainToDetailsPb(details), - NextStep: &user.StartIdentityProviderIntentResponse_PostForm{ - PostForm: []byte(content), - }, - }, nil } + return &user.StartIdentityProviderIntentResponse{ + Details: object.DomainToDetailsPb(details), + NextStep: &user.StartIdentityProviderIntentResponse_PostForm{ + PostForm: []byte(content), + }, + }, nil } func (s *Server) startLDAPIntent(ctx context.Context, idpID string, ldapCredentials *user.LDAPCredentials) (*user.StartIdentityProviderIntentResponse, error) { - intentWriteModel, details, err := s.command.CreateIntent(ctx, idpID, "", "", authz.GetInstance(ctx).InstanceID()) + intentWriteModel, details, err := s.command.CreateIntent(ctx, "", idpID, "", "", authz.GetInstance(ctx).InstanceID(), nil) if err != nil { return nil, err } diff --git a/internal/api/idp/idp.go b/internal/api/idp/idp.go index 01594c43ba..c3e9586a59 100644 --- a/internal/api/idp/idp.go +++ b/internal/api/idp/idp.go @@ -328,7 +328,7 @@ func (h *Handler) handleCallback(w http.ResponseWriter, r *http.Request) { return } - idpUser, idpSession, err := h.fetchIDPUserFromCode(ctx, provider, data.Code, data.User) + idpUser, idpSession, err := h.fetchIDPUserFromCode(ctx, provider, data.Code, data.User, intent.IDPArguments) if err != nil { cmdErr := h.commands.FailIDPIntent(ctx, intent, err.Error()) logging.WithFields("intent", intent.AggregateID).OnError(cmdErr).Error("failed to push failed event on idp intent") @@ -410,23 +410,23 @@ func redirectToFailureURL(w http.ResponseWriter, r *http.Request, i *command.IDP http.Redirect(w, r, i.FailureURL.String(), http.StatusFound) } -func (h *Handler) fetchIDPUserFromCode(ctx context.Context, identityProvider idp.Provider, code string, appleUser string) (user idp.User, idpTokens idp.Session, err error) { +func (h *Handler) fetchIDPUserFromCode(ctx context.Context, identityProvider idp.Provider, code string, appleUser string, idpArguments map[string]any) (user idp.User, idpTokens idp.Session, err error) { var session idp.Session switch provider := identityProvider.(type) { case *oauth.Provider: - session = &oauth.Session{Provider: provider, Code: code} + session = oauth.NewSession(provider, code, idpArguments) case *openid.Provider: - session = &openid.Session{Provider: provider, Code: code} + session = openid.NewSession(provider, code, idpArguments) case *azuread.Provider: - session = &azuread.Session{Provider: provider, Code: code} + session = azuread.NewSession(provider, code) case *github.Provider: - session = &oauth.Session{Provider: provider.Provider, Code: code} + session = oauth.NewSession(provider.Provider, code, idpArguments) case *gitlab.Provider: - session = &openid.Session{Provider: provider.Provider, Code: code} + session = openid.NewSession(provider.Provider, code, idpArguments) case *google.Provider: - session = &openid.Session{Provider: provider.Provider, Code: code} + session = openid.NewSession(provider.Provider, code, idpArguments) case *apple.Provider: - session = &apple.Session{Session: &openid.Session{Provider: provider.Provider, Code: code}, UserFormValue: appleUser} + session = apple.NewSession(provider, code, appleUser) case *jwt.Provider, *ldap.Provider, *saml2.Provider: return nil, nil, zerrors.ThrowInvalidArgument(nil, "IDP-52jmn", "Errors.ExternalIDP.IDPTypeNotImplemented") default: diff --git a/internal/api/ui/login/external_provider_handler.go b/internal/api/ui/login/external_provider_handler.go index 5d0e0ede67..649c8a958e 100644 --- a/internal/api/ui/login/external_provider_handler.go +++ b/internal/api/ui/login/external_provider_handler.go @@ -149,14 +149,7 @@ func (l *Login) handleIDP(w http.ResponseWriter, r *http.Request, authReq *domai l.renderError(w, r, authReq, err) return } - userAgentID, _ := http_mw.UserAgentIDFromCtx(r.Context()) - err = l.authRepo.SelectExternalIDP(r.Context(), authReq.ID, identityProvider.ID, userAgentID) - if err != nil { - l.externalAuthFailed(w, r, authReq, err) - return - } var provider idp.Provider - switch identityProvider.Type { case domain.IDPTypeOAuth: provider, err = l.oauthProvider(r.Context(), identityProvider) @@ -199,6 +192,13 @@ func (l *Login) handleIDP(w http.ResponseWriter, r *http.Request, authReq *domai return } + userAgentID, _ := http_mw.UserAgentIDFromCtx(r.Context()) + err = l.authRepo.SelectExternalIDP(r.Context(), authReq.ID, identityProvider.ID, userAgentID, session.PersistentParameters()) + if err != nil { + l.externalAuthFailed(w, r, authReq, err) + return + } + content, redirect := session.GetAuth(r.Context()) if redirect { http.Redirect(w, r, content, http.StatusFound) @@ -271,79 +271,78 @@ func (l *Login) handleExternalLoginCallback(w http.ResponseWriter, r *http.Reque l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return } - var provider idp.Provider var session idp.Session switch identityProvider.Type { case domain.IDPTypeOAuth: - provider, err = l.oauthProvider(r.Context(), identityProvider) + provider, err := l.oauthProvider(r.Context(), identityProvider) if err != nil { l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return } - session = &oauth.Session{Provider: provider.(*oauth.Provider), Code: data.Code} + session = oauth.NewSession(provider, data.Code, authReq.SelectedIDPConfigArgs) case domain.IDPTypeOIDC: - provider, err = l.oidcProvider(r.Context(), identityProvider) + provider, err := l.oidcProvider(r.Context(), identityProvider) if err != nil { l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return } - session = &openid.Session{Provider: provider.(*openid.Provider), Code: data.Code} + session = openid.NewSession(provider, data.Code, authReq.SelectedIDPConfigArgs) case domain.IDPTypeAzureAD: - provider, err = l.azureProvider(r.Context(), identityProvider) + provider, err := l.azureProvider(r.Context(), identityProvider) if err != nil { l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return } - session = &azuread.Session{Provider: provider.(*azuread.Provider), Code: data.Code} + session = azuread.NewSession(provider, data.Code) case domain.IDPTypeGitHub: - provider, err = l.githubProvider(r.Context(), identityProvider) + provider, err := l.githubProvider(r.Context(), identityProvider) if err != nil { l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return } - session = &oauth.Session{Provider: provider.(*github.Provider).Provider, Code: data.Code} + session = oauth.NewSession(provider.Provider, data.Code, authReq.SelectedIDPConfigArgs) case domain.IDPTypeGitHubEnterprise: - provider, err = l.githubEnterpriseProvider(r.Context(), identityProvider) + provider, err := l.githubEnterpriseProvider(r.Context(), identityProvider) if err != nil { l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return } - session = &oauth.Session{Provider: provider.(*github.Provider).Provider, Code: data.Code} + session = oauth.NewSession(provider.Provider, data.Code, authReq.SelectedIDPConfigArgs) case domain.IDPTypeGitLab: - provider, err = l.gitlabProvider(r.Context(), identityProvider) + provider, err := l.gitlabProvider(r.Context(), identityProvider) if err != nil { l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return } - session = &openid.Session{Provider: provider.(*gitlab.Provider).Provider, Code: data.Code} + session = openid.NewSession(provider.Provider, data.Code, authReq.SelectedIDPConfigArgs) case domain.IDPTypeGitLabSelfHosted: - provider, err = l.gitlabSelfHostedProvider(r.Context(), identityProvider) + provider, err := l.gitlabSelfHostedProvider(r.Context(), identityProvider) if err != nil { l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return } - session = &openid.Session{Provider: provider.(*gitlab.Provider).Provider, Code: data.Code} + session = openid.NewSession(provider.Provider, data.Code, authReq.SelectedIDPConfigArgs) case domain.IDPTypeGoogle: - provider, err = l.googleProvider(r.Context(), identityProvider) + provider, err := l.googleProvider(r.Context(), identityProvider) if err != nil { l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return } - session = &openid.Session{Provider: provider.(*google.Provider).Provider, Code: data.Code} + session = openid.NewSession(provider.Provider, data.Code, authReq.SelectedIDPConfigArgs) case domain.IDPTypeApple: - provider, err = l.appleProvider(r.Context(), identityProvider) + provider, err := l.appleProvider(r.Context(), identityProvider) if err != nil { l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return } - session = &apple.Session{Session: &openid.Session{Provider: provider.(*apple.Provider).Provider, Code: data.Code}, UserFormValue: data.User} + session = apple.NewSession(provider, data.Code, data.User) case domain.IDPTypeSAML: - provider, err = l.samlProvider(r.Context(), identityProvider) + provider, err := l.samlProvider(r.Context(), identityProvider) if err != nil { l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return } - session, err = saml.NewSession(provider.(*saml.Provider), authReq.SAMLRequestID, r) + session, err = saml.NewSession(provider, authReq.SAMLRequestID, r) if err != nil { l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return @@ -440,7 +439,7 @@ func (l *Login) handleExternalUserAuthenticated( ) { externalUser := mapIDPUserToExternalUser(user, provider.ID) // ensure the linked IDP is added to the login policy - if err := l.authRepo.SelectExternalIDP(r.Context(), authReq.ID, provider.ID, authReq.AgentID); err != nil { + if err := l.authRepo.SelectExternalIDP(r.Context(), authReq.ID, provider.ID, authReq.AgentID, authReq.SelectedIDPConfigArgs); err != nil { l.renderError(w, r, authReq, err) return } @@ -1005,11 +1004,17 @@ func (l *Login) oidcProvider(ctx context.Context, identityProvider *query.IDPTem if err != nil { return nil, err } - opts := make([]openid.ProviderOpts, 1, 2) + opts := make([]openid.ProviderOpts, 1, 3) opts[0] = openid.WithSelectAccount() if identityProvider.OIDCIDPTemplate.IsIDTokenMapping { opts = append(opts, openid.WithIDTokenMapping()) } + + if identityProvider.OIDCIDPTemplate.UsePKCE { + // we do not pass any cookie handler, since we store the verifier internally, rather than in a cookie + opts = append(opts, openid.WithRelyingPartyOption(rp.WithPKCE(nil))) + } + return openid.New(identityProvider.Name, identityProvider.OIDCIDPTemplate.Issuer, identityProvider.OIDCIDPTemplate.ClientID, @@ -1047,6 +1052,12 @@ func (l *Login) oauthProvider(ctx context.Context, identityProvider *query.IDPTe RedirectURL: l.baseURL(ctx) + EndpointExternalLoginCallback, Scopes: identityProvider.OAuthIDPTemplate.Scopes, } + + opts := make([]oauth.ProviderOpts, 0, 1) + if identityProvider.OAuthIDPTemplate.UsePKCE { + // we do not pass any cookie handler, since we store the verifier internally, rather than in a cookie + opts = append(opts, oauth.WithRelyingPartyOption(rp.WithPKCE(nil))) + } return oauth.New( config, identityProvider.Name, @@ -1054,6 +1065,7 @@ func (l *Login) oauthProvider(ctx context.Context, identityProvider *query.IDPTe func() idp.User { return oauth.NewUserMapper(identityProvider.OAuthIDPTemplate.IDAttribute) }, + opts..., ) } diff --git a/internal/api/ui/login/jwt_handler.go b/internal/api/ui/login/jwt_handler.go index 7c643e9a43..bff316252f 100644 --- a/internal/api/ui/login/jwt_handler.go +++ b/internal/api/ui/login/jwt_handler.go @@ -81,7 +81,7 @@ func (l *Login) handleJWTExtraction(w http.ResponseWriter, r *http.Request, auth l.renderError(w, r, authReq, err) return } - session := &jwt.Session{Provider: provider, Tokens: &oidc.Tokens[*oidc.IDTokenClaims]{IDToken: token, Token: &oauth2.Token{}}} + session := jwt.NewSession(provider, &oidc.Tokens[*oidc.IDTokenClaims]{IDToken: token, Token: &oauth2.Token{}}) user, err := session.FetchUser(r.Context()) if err != nil { if _, _, actionErr := l.runPostExternalAuthenticationActions(new(domain.ExternalUser), tokens(session), authReq, r, user, err); actionErr != nil { diff --git a/internal/api/ui/login/ldap_handler.go b/internal/api/ui/login/ldap_handler.go index 147a319523..4703a462bf 100644 --- a/internal/api/ui/login/ldap_handler.go +++ b/internal/api/ui/login/ldap_handler.go @@ -66,7 +66,7 @@ func (l *Login) handleLDAPCallback(w http.ResponseWriter, r *http.Request) { l.renderLDAPLogin(w, r, authReq, err) return } - session := &ldap.Session{Provider: provider, User: data.Username, Password: data.Password} + session := ldap.NewSession(provider, data.Username, data.Password) user, err := session.FetchUser(r.Context()) if err != nil { diff --git a/internal/auth/repository/auth_request.go b/internal/auth/repository/auth_request.go index c16a757a01..53272d2d2f 100644 --- a/internal/auth/repository/auth_request.go +++ b/internal/auth/repository/auth_request.go @@ -20,7 +20,7 @@ type AuthRequestRepository interface { SetExternalUserLogin(ctx context.Context, authReqID, userAgentID string, user *domain.ExternalUser) error SetLinkingUser(ctx context.Context, request *domain.AuthRequest, externalUser *domain.ExternalUser) error SelectUser(ctx context.Context, authReqID, userID, userAgentID string) error - SelectExternalIDP(ctx context.Context, authReqID, idpConfigID, userAgentID string) error + SelectExternalIDP(ctx context.Context, authReqID, idpConfigID, userAgentID string, idpArguments map[string]any) error VerifyPassword(ctx context.Context, id, userID, resourceOwner, password, userAgentID string, info *domain.BrowserInfo) error VerifyMFAOTP(ctx context.Context, authRequestID, userID, resourceOwner, code, userAgentID string, info *domain.BrowserInfo) error diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request.go b/internal/auth/repository/eventsourcing/eventstore/auth_request.go index 60486b66f9..c8ae92b418 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request.go @@ -255,14 +255,14 @@ func (repo *AuthRequestRepo) CheckLoginName(ctx context.Context, id, loginName, return repo.AuthRequests.UpdateAuthRequest(ctx, request) } -func (repo *AuthRequestRepo) SelectExternalIDP(ctx context.Context, authReqID, idpConfigID, userAgentID string) (err error) { +func (repo *AuthRequestRepo) SelectExternalIDP(ctx context.Context, authReqID, idpConfigID, userAgentID string, idpArguments map[string]any) (err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() request, err := repo.getAuthRequest(ctx, authReqID, userAgentID) if err != nil { return err } - err = repo.checkSelectedExternalIDP(request, idpConfigID) + err = repo.checkSelectedExternalIDP(request, idpConfigID, idpArguments) if err != nil { return err } @@ -984,10 +984,11 @@ func queryLoginPolicyToDomain(policy *query.LoginPolicy) *domain.LoginPolicy { } } -func (repo *AuthRequestRepo) checkSelectedExternalIDP(request *domain.AuthRequest, idpConfigID string) error { +func (repo *AuthRequestRepo) checkSelectedExternalIDP(request *domain.AuthRequest, idpConfigID string, idpArguments map[string]any) error { for _, externalIDP := range request.AllowedExternalIDPs { if externalIDP.IDPConfigID == idpConfigID { request.SelectedIDPConfigID = idpConfigID + request.SelectedIDPConfigArgs = idpArguments return nil } } diff --git a/internal/command/idp.go b/internal/command/idp.go index b9185cbc57..821a577900 100644 --- a/internal/command/idp.go +++ b/internal/command/idp.go @@ -20,6 +20,7 @@ type GenericOAuthProvider struct { UserEndpoint string Scopes []string IDAttribute string + UsePKCE bool IDPOptions idp.Options } @@ -30,6 +31,7 @@ type GenericOIDCProvider struct { ClientSecret string Scopes []string IsIDTokenMapping bool + UsePKCE bool IDPOptions idp.Options } diff --git a/internal/command/idp_intent.go b/internal/command/idp_intent.go index 483cdcd08e..3cd9991679 100644 --- a/internal/command/idp_intent.go +++ b/internal/command/idp_intent.go @@ -25,7 +25,7 @@ import ( "github.com/zitadel/zitadel/internal/zerrors" ) -func (c *Commands) prepareCreateIntent(writeModel *IDPIntentWriteModel, idpID, successURL, failureURL string) preparation.Validation { +func (c *Commands) prepareCreateIntent(writeModel *IDPIntentWriteModel, idpID, successURL, failureURL string, idpArguments map[string]any) preparation.Validation { return func() (_ preparation.CreateCommands, err error) { if idpID == "" { return nil, zerrors.ThrowInvalidArgument(nil, "COMMAND-x8j2bk", "Errors.Intent.IDPMissing") @@ -53,24 +53,25 @@ func (c *Commands) prepareCreateIntent(writeModel *IDPIntentWriteModel, idpID, s successURL, failureURL, idpID, + idpArguments, ), }, nil }, nil } } -func (c *Commands) CreateIntent(ctx context.Context, idpID, successURL, failureURL, resourceOwner string) (*IDPIntentWriteModel, *domain.ObjectDetails, error) { - id, err := c.idGenerator.Next() - if err != nil { - return nil, nil, err - } - writeModel := NewIDPIntentWriteModel(id, resourceOwner) - if err != nil { - return nil, nil, err +func (c *Commands) CreateIntent(ctx context.Context, intentID, idpID, successURL, failureURL, resourceOwner string, idpArguments map[string]any) (*IDPIntentWriteModel, *domain.ObjectDetails, error) { + if intentID == "" { + var err error + intentID, err = c.idGenerator.Next() + if err != nil { + return nil, nil, err + } } + writeModel := NewIDPIntentWriteModel(intentID, resourceOwner) //nolint: staticcheck - cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, c.prepareCreateIntent(writeModel, idpID, successURL, failureURL)) + cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, c.prepareCreateIntent(writeModel, idpID, successURL, failureURL, idpArguments)) if err != nil { return nil, nil, err } @@ -132,18 +133,17 @@ func (c *Commands) GetActiveIntent(ctx context.Context, intentID string) (*IDPIn return intent, nil } -func (c *Commands) AuthFromProvider(ctx context.Context, idpID, state string, idpCallback, samlRootURL string) (string, bool, error) { +func (c *Commands) AuthFromProvider(ctx context.Context, idpID, idpCallback, samlRootURL string) (state string, session idp.Session, err error) { + state, err = c.idGenerator.Next() + if err != nil { + return "", nil, err + } provider, err := c.GetProvider(ctx, idpID, idpCallback, samlRootURL) if err != nil { - return "", false, err + return "", nil, err } - session, err := provider.BeginAuth(ctx, state) - if err != nil { - return "", false, err - } - - content, redirect := session.GetAuth(ctx) - return content, redirect, nil + session, err = provider.BeginAuth(ctx, state) + return state, session, err } func getIDPIntentWriteModel(ctx context.Context, writeModel *IDPIntentWriteModel, filter preparation.FilterToQueryReducer) error { diff --git a/internal/command/idp_intent_model.go b/internal/command/idp_intent_model.go index 62794323e1..c6bc26ab06 100644 --- a/internal/command/idp_intent_model.go +++ b/internal/command/idp_intent_model.go @@ -12,13 +12,14 @@ import ( type IDPIntentWriteModel struct { eventstore.WriteModel - SuccessURL *url.URL - FailureURL *url.URL - IDPID string - IDPUser []byte - IDPUserID string - IDPUserName string - UserID string + SuccessURL *url.URL + FailureURL *url.URL + IDPID string + IDPArguments map[string]any + IDPUser []byte + IDPUserID string + IDPUserName string + UserID string IDPAccessToken *crypto.CryptoValue IDPIDToken string @@ -81,6 +82,7 @@ func (wm *IDPIntentWriteModel) reduceStartedEvent(e *idpintent.StartedEvent) { wm.SuccessURL = e.SuccessURL wm.FailureURL = e.FailureURL wm.IDPID = e.IDPID + wm.IDPArguments = e.IDPArguments wm.State = domain.IDPIntentStateStarted } diff --git a/internal/command/idp_intent_test.go b/internal/command/idp_intent_test.go index 832e2e9902..2400b9ee35 100644 --- a/internal/command/idp_intent_test.go +++ b/internal/command/idp_intent_test.go @@ -39,11 +39,13 @@ func TestCommands_CreateIntent(t *testing.T) { idGenerator id.Generator } type args struct { - ctx context.Context - idpID string - successURL string - failureURL string - instanceID string + ctx context.Context + intentID string + idpID string + successURL string + failureURL string + instanceID string + idpArguments map[string]any } type res struct { intentID string @@ -182,6 +184,7 @@ func TestCommands_CreateIntent(t *testing.T) { "user", "idAttribute", nil, + true, rep_idp.Options{}, )), ), @@ -195,6 +198,7 @@ func TestCommands_CreateIntent(t *testing.T) { success, failure, "idp", + nil, ) }(), ), @@ -235,6 +239,7 @@ func TestCommands_CreateIntent(t *testing.T) { "user", "idAttribute", nil, + true, rep_idp.Options{}, )), ), @@ -248,6 +253,9 @@ func TestCommands_CreateIntent(t *testing.T) { success, failure, "idp", + map[string]interface{}{ + "verifier": "pkceOAuthVerifier", + }, ) }(), ), @@ -260,6 +268,9 @@ func TestCommands_CreateIntent(t *testing.T) { idpID: "idp", successURL: "https://success.url", failureURL: "https://failure.url", + idpArguments: map[string]interface{}{ + "verifier": "pkceOAuthVerifier", + }, }, res{ intentID: "id", @@ -288,6 +299,7 @@ func TestCommands_CreateIntent(t *testing.T) { "user", "idAttribute", nil, + true, rep_idp.Options{}, )), ), @@ -301,6 +313,9 @@ func TestCommands_CreateIntent(t *testing.T) { success, failure, "idp", + map[string]interface{}{ + "verifier": "pkceOAuthVerifier", + }, ) }(), ), @@ -313,6 +328,69 @@ func TestCommands_CreateIntent(t *testing.T) { idpID: "idp", successURL: "https://success.url", failureURL: "https://failure.url", + idpArguments: map[string]interface{}{ + "verifier": "pkceOAuthVerifier", + }, + }, + res{ + intentID: "id", + details: &domain.ObjectDetails{ResourceOwner: "instance"}, + }, + }, + { + "push, with id", + fields{ + eventstore: expectEventstore( + expectFilter(), + expectFilter( + eventFromEventPusher( + instance.NewOAuthIDPAddedEvent(context.Background(), &instance.NewAggregate("instance").Aggregate, + "idp", + "name", + "clientID", + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("clientSecret"), + }, + "auth", + "token", + "user", + "idAttribute", + nil, + true, + rep_idp.Options{}, + )), + ), + expectPush( + func() eventstore.Command { + success, _ := url.Parse("https://success.url") + failure, _ := url.Parse("https://failure.url") + return idpintent.NewStartedEvent( + context.Background(), + &idpintent.NewAggregate("id", "instance").Aggregate, + success, + failure, + "idp", + map[string]interface{}{ + "verifier": "pkceOAuthVerifier", + }, + ) + }(), + ), + ), + }, + args{ + ctx: context.Background(), + instanceID: "instance", + intentID: "id", + idpID: "idp", + successURL: "https://success.url", + failureURL: "https://failure.url", + idpArguments: map[string]interface{}{ + "verifier": "pkceOAuthVerifier", + }, }, res{ intentID: "id", @@ -326,7 +404,7 @@ func TestCommands_CreateIntent(t *testing.T) { eventstore: tt.fields.eventstore(t), idGenerator: tt.fields.idGenerator, } - intentWriteModel, details, err := c.CreateIntent(tt.args.ctx, tt.args.idpID, tt.args.successURL, tt.args.failureURL, tt.args.instanceID) + intentWriteModel, details, err := c.CreateIntent(tt.args.ctx, tt.args.intentID, tt.args.idpID, tt.args.successURL, tt.args.failureURL, tt.args.instanceID, tt.args.idpArguments) require.ErrorIs(t, err, tt.res.err) if intentWriteModel != nil { assert.Equal(t, tt.res.intentID, intentWriteModel.AggregateID) @@ -342,11 +420,11 @@ func TestCommands_AuthFromProvider(t *testing.T) { type fields struct { eventstore func(t *testing.T) *eventstore.Eventstore secretCrypto crypto.EncryptionAlgorithm + idGenerator id.Generator } type args struct { ctx context.Context idpID string - state string callbackURL string samlRootURL string } @@ -361,6 +439,22 @@ func TestCommands_AuthFromProvider(t *testing.T) { args args res res }{ + { + "error no id generator", + fields{ + secretCrypto: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + eventstore: expectEventstore(), + idGenerator: mock.NewIDGeneratorExpectError(t, zerrors.ThrowInternal(nil, "", "error id")), + }, + args{ + ctx: authz.SetCtxData(context.Background(), authz.CtxData{OrgID: "ro"}), + idpID: "idp", + callbackURL: "url", + }, + res{ + err: zerrors.ThrowInternal(nil, "", "error id"), + }, + }, { "idp not existing", fields{ @@ -368,11 +462,11 @@ func TestCommands_AuthFromProvider(t *testing.T) { eventstore: expectEventstore( expectFilter(), ), + idGenerator: mock.ExpectID(t, "id"), }, args{ ctx: authz.SetCtxData(context.Background(), authz.CtxData{OrgID: "ro"}), idpID: "idp", - state: "state", callbackURL: "url", }, res{ @@ -402,6 +496,7 @@ func TestCommands_AuthFromProvider(t *testing.T) { "user", "idAttribute", nil, + true, rep_idp.Options{}, )), eventFromEventPusherWithInstanceID( @@ -412,11 +507,11 @@ func TestCommands_AuthFromProvider(t *testing.T) { ), ), ), + idGenerator: mock.ExpectID(t, "id"), }, args{ ctx: authz.SetCtxData(context.Background(), authz.CtxData{OrgID: "ro"}), idpID: "idp", - state: "state", callbackURL: "url", }, res{ @@ -446,6 +541,7 @@ func TestCommands_AuthFromProvider(t *testing.T) { "user", "idAttribute", nil, + true, rep_idp.Options{}, )), ), @@ -467,19 +563,20 @@ func TestCommands_AuthFromProvider(t *testing.T) { "user", "idAttribute", nil, + true, rep_idp.Options{}, )), ), ), + idGenerator: mock.ExpectID(t, "id"), }, args{ ctx: authz.SetCtxData(context.Background(), authz.CtxData{OrgID: "ro"}), idpID: "idp", - state: "state", callbackURL: "url", }, res{ - content: "auth?client_id=clientID&prompt=select_account&redirect_uri=url&response_type=code&state=state", + content: "auth?client_id=clientID&prompt=select_account&redirect_uri=url&response_type=code&state=id", redirect: true, }, }, @@ -504,6 +601,7 @@ func TestCommands_AuthFromProvider(t *testing.T) { }, []string{"openid", "profile", "User.Read"}, false, + true, rep_idp.Options{}, )), eventFromEventPusherWithInstanceID( @@ -540,6 +638,7 @@ func TestCommands_AuthFromProvider(t *testing.T) { }, []string{"openid", "profile", "User.Read"}, false, + true, rep_idp.Options{}, )), eventFromEventPusherWithInstanceID( @@ -561,15 +660,15 @@ func TestCommands_AuthFromProvider(t *testing.T) { )), ), ), + idGenerator: mock.ExpectID(t, "id"), }, args{ ctx: authz.SetCtxData(context.Background(), authz.CtxData{OrgID: "ro"}), idpID: "idp", - state: "state", callbackURL: "url", }, res{ - content: "https://login.microsoftonline.com/tenant/oauth2/v2.0/authorize?client_id=clientID&prompt=select_account&redirect_uri=url&response_type=code&scope=openid+profile+User.Read&state=state", + content: "https://login.microsoftonline.com/tenant/oauth2/v2.0/authorize?client_id=clientID&prompt=select_account&redirect_uri=url&response_type=code&scope=openid+profile+User.Read&state=id", redirect: true, }, }, @@ -579,9 +678,16 @@ func TestCommands_AuthFromProvider(t *testing.T) { c := &Commands{ eventstore: tt.fields.eventstore(t), idpConfigEncryption: tt.fields.secretCrypto, + idGenerator: tt.fields.idGenerator, } - content, redirect, err := c.AuthFromProvider(tt.args.ctx, tt.args.idpID, tt.args.state, tt.args.callbackURL, tt.args.samlRootURL) + _, session, err := c.AuthFromProvider(tt.args.ctx, tt.args.idpID, tt.args.callbackURL, tt.args.samlRootURL) require.ErrorIs(t, err, tt.res.err) + + var content string + var redirect bool + if err == nil { + content, redirect = session.GetAuth(tt.args.ctx) + } assert.Equal(t, tt.res.redirect, redirect) assert.Equal(t, tt.res.content, content) }) @@ -592,11 +698,11 @@ func TestCommands_AuthFromProvider_SAML(t *testing.T) { type fields struct { eventstore func(t *testing.T) *eventstore.Eventstore secretCrypto crypto.EncryptionAlgorithm + idGenerator id.Generator } type args struct { ctx context.Context idpID string - state string callbackURL string samlRootURL string } @@ -669,6 +775,7 @@ func TestCommands_AuthFromProvider_SAML(t *testing.T) { success, failure, "idp", + nil, ) }(), ), @@ -683,11 +790,11 @@ func TestCommands_AuthFromProvider_SAML(t *testing.T) { }, ), ), + idGenerator: mock.ExpectID(t, "id"), }, args{ ctx: authz.SetCtxData(context.Background(), authz.CtxData{OrgID: "ro"}), idpID: "idp", - state: "id", callbackURL: "url", samlRootURL: "samlurl", }, @@ -705,10 +812,12 @@ func TestCommands_AuthFromProvider_SAML(t *testing.T) { c := &Commands{ eventstore: tt.fields.eventstore(t), idpConfigEncryption: tt.fields.secretCrypto, + idGenerator: tt.fields.idGenerator, } - content, _, err := c.AuthFromProvider(tt.args.ctx, tt.args.idpID, tt.args.state, tt.args.callbackURL, tt.args.samlRootURL) + _, session, err := c.AuthFromProvider(tt.args.ctx, tt.args.idpID, tt.args.callbackURL, tt.args.samlRootURL) require.ErrorIs(t, err, tt.res.err) + content, _ := session.GetAuth(tt.args.ctx) authURL, err := url.Parse(content) require.NoError(t, err) diff --git a/internal/command/idp_model.go b/internal/command/idp_model.go index 7a51ea1112..5257d38bf4 100644 --- a/internal/command/idp_model.go +++ b/internal/command/idp_model.go @@ -45,6 +45,7 @@ type OAuthIDPWriteModel struct { UserEndpoint string Scopes []string IDAttribute string + UsePKCE bool idp.Options State domain.IDPState @@ -73,6 +74,7 @@ func (wm *OAuthIDPWriteModel) reduceAddedEvent(e *idp.OAuthIDPAddedEvent) { wm.UserEndpoint = e.UserEndpoint wm.Scopes = e.Scopes wm.IDAttribute = e.IDAttribute + wm.UsePKCE = e.UsePKCE wm.Options = e.Options wm.State = domain.IDPStateActive } @@ -102,6 +104,9 @@ func (wm *OAuthIDPWriteModel) reduceChangedEvent(e *idp.OAuthIDPChangedEvent) { if e.IDAttribute != nil { wm.IDAttribute = *e.IDAttribute } + if e.UsePKCE != nil { + wm.UsePKCE = *e.UsePKCE + } wm.Options.ReduceChanges(e.OptionChanges) } @@ -115,6 +120,7 @@ func (wm *OAuthIDPWriteModel) NewChanges( userEndpoint, idAttribute string, scopes []string, + usePKCE bool, options idp.Options, ) ([]idp.OAuthIDPChanges, error) { changes := make([]idp.OAuthIDPChanges, 0) @@ -148,6 +154,9 @@ func (wm *OAuthIDPWriteModel) NewChanges( if wm.IDAttribute != idAttribute { changes = append(changes, idp.ChangeOAuthIDAttribute(idAttribute)) } + if wm.UsePKCE != usePKCE { + changes = append(changes, idp.ChangeOAuthUsePKCE(usePKCE)) + } opts := wm.Options.Changes(options) if !opts.IsZero() { changes = append(changes, idp.ChangeOAuthOptions(opts)) @@ -208,6 +217,7 @@ type OIDCIDPWriteModel struct { ClientSecret *crypto.CryptoValue Scopes []string IsIDTokenMapping bool + UsePKCE bool idp.Options State domain.IDPState @@ -248,6 +258,7 @@ func (wm *OIDCIDPWriteModel) reduceAddedEvent(e *idp.OIDCIDPAddedEvent) { wm.ClientSecret = e.ClientSecret wm.Scopes = e.Scopes wm.IsIDTokenMapping = e.IsIDTokenMapping + wm.UsePKCE = e.UsePKCE wm.Options = e.Options wm.State = domain.IDPStateActive } @@ -271,6 +282,9 @@ func (wm *OIDCIDPWriteModel) reduceChangedEvent(e *idp.OIDCIDPChangedEvent) { if e.IsIDTokenMapping != nil { wm.IsIDTokenMapping = *e.IsIDTokenMapping } + if e.UsePKCE != nil { + wm.UsePKCE = *e.UsePKCE + } wm.Options.ReduceChanges(e.OptionChanges) } @@ -281,7 +295,7 @@ func (wm *OIDCIDPWriteModel) NewChanges( clientSecretString string, secretCrypto crypto.EncryptionAlgorithm, scopes []string, - idTokenMapping bool, + idTokenMapping, usePKCE bool, options idp.Options, ) ([]idp.OIDCIDPChanges, error) { changes := make([]idp.OIDCIDPChanges, 0) @@ -309,6 +323,9 @@ func (wm *OIDCIDPWriteModel) NewChanges( if wm.IsIDTokenMapping != idTokenMapping { changes = append(changes, idp.ChangeOIDCIsIDTokenMapping(idTokenMapping)) } + if wm.UsePKCE != usePKCE { + changes = append(changes, idp.ChangeOIDCUsePKCE(usePKCE)) + } opts := wm.Options.Changes(options) if !opts.IsZero() { changes = append(changes, idp.ChangeOIDCOptions(opts)) diff --git a/internal/command/instance_idp.go b/internal/command/instance_idp.go index 46f54cad62..cea850dc0e 100644 --- a/internal/command/instance_idp.go +++ b/internal/command/instance_idp.go @@ -656,6 +656,7 @@ func (c *Commands) prepareAddInstanceOAuthProvider(a *instance.Aggregate, writeM provider.UserEndpoint, provider.IDAttribute, provider.Scopes, + provider.UsePKCE, provider.IDPOptions, ), }, nil @@ -711,6 +712,7 @@ func (c *Commands) prepareUpdateInstanceOAuthProvider(a *instance.Aggregate, wri provider.UserEndpoint, provider.IDAttribute, provider.Scopes, + provider.UsePKCE, provider.IDPOptions, ) if err != nil || event == nil { @@ -759,6 +761,7 @@ func (c *Commands) prepareAddInstanceOIDCProvider(a *instance.Aggregate, writeMo secret, provider.Scopes, provider.IsIDTokenMapping, + provider.UsePKCE, provider.IDPOptions, ), }, nil @@ -803,6 +806,7 @@ func (c *Commands) prepareUpdateInstanceOIDCProvider(a *instance.Aggregate, writ c.idpConfigEncryption, provider.Scopes, provider.IsIDTokenMapping, + provider.UsePKCE, provider.IDPOptions, ) if err != nil || event == nil { diff --git a/internal/command/instance_idp_model.go b/internal/command/instance_idp_model.go index 68f060dd26..d94c19d318 100644 --- a/internal/command/instance_idp_model.go +++ b/internal/command/instance_idp_model.go @@ -68,6 +68,7 @@ func (wm *InstanceOAuthIDPWriteModel) NewChangedEvent( userEndpoint, idAttribute string, scopes []string, + usePKCE bool, options idp.Options, ) (*instance.OAuthIDPChangedEvent, error) { @@ -81,6 +82,7 @@ func (wm *InstanceOAuthIDPWriteModel) NewChangedEvent( userEndpoint, idAttribute, scopes, + usePKCE, options, ) if err != nil || len(changes) == 0 { @@ -174,7 +176,7 @@ func (wm *InstanceOIDCIDPWriteModel) NewChangedEvent( clientSecretString string, secretCrypto crypto.EncryptionAlgorithm, scopes []string, - idTokenMapping bool, + idTokenMapping, usePKCE bool, options idp.Options, ) (*instance.OIDCIDPChangedEvent, error) { @@ -186,6 +188,7 @@ func (wm *InstanceOIDCIDPWriteModel) NewChangedEvent( secretCrypto, scopes, idTokenMapping, + usePKCE, options, ) if err != nil || len(changes) == 0 { diff --git a/internal/command/instance_idp_test.go b/internal/command/instance_idp_test.go index 39eb74815b..8d872561c2 100644 --- a/internal/command/instance_idp_test.go +++ b/internal/command/instance_idp_test.go @@ -270,6 +270,7 @@ func TestCommandSide_AddInstanceGenericOAuthIDP(t *testing.T) { "user", "idAttribute", nil, + true, idp.Options{}, ), ), @@ -287,6 +288,7 @@ func TestCommandSide_AddInstanceGenericOAuthIDP(t *testing.T) { TokenEndpoint: "token", UserEndpoint: "user", IDAttribute: "idAttribute", + UsePKCE: true, }, }, res: res{ @@ -315,6 +317,7 @@ func TestCommandSide_AddInstanceGenericOAuthIDP(t *testing.T) { "user", "idAttribute", []string{"user"}, + true, idp.Options{ IsCreationAllowed: true, IsLinkingAllowed: true, @@ -338,6 +341,7 @@ func TestCommandSide_AddInstanceGenericOAuthIDP(t *testing.T) { UserEndpoint: "user", Scopes: []string{"user"}, IDAttribute: "idAttribute", + UsePKCE: true, IDPOptions: idp.Options{ IsCreationAllowed: true, IsLinkingAllowed: true, @@ -569,6 +573,7 @@ func TestCommandSide_UpdateInstanceGenericOAuthIDP(t *testing.T) { "user", "idAttribute", nil, + true, idp.Options{}, )), ), @@ -584,6 +589,7 @@ func TestCommandSide_UpdateInstanceGenericOAuthIDP(t *testing.T) { TokenEndpoint: "token", UserEndpoint: "user", IDAttribute: "idAttribute", + UsePKCE: true, }, }, res: res{ @@ -611,6 +617,7 @@ func TestCommandSide_UpdateInstanceGenericOAuthIDP(t *testing.T) { "user", "idAttribute", nil, + false, idp.Options{}, )), ), @@ -633,6 +640,7 @@ func TestCommandSide_UpdateInstanceGenericOAuthIDP(t *testing.T) { idp.ChangeOAuthUserEndpoint("new user"), idp.ChangeOAuthScopes([]string{"openid", "profile"}), idp.ChangeOAuthIDAttribute("newAttribute"), + idp.ChangeOAuthUsePKCE(true), idp.ChangeOAuthOptions(idp.OptionChanges{ IsCreationAllowed: &t, IsLinkingAllowed: &t, @@ -659,6 +667,7 @@ func TestCommandSide_UpdateInstanceGenericOAuthIDP(t *testing.T) { UserEndpoint: "new user", Scopes: []string{"openid", "profile"}, IDAttribute: "newAttribute", + UsePKCE: true, IDPOptions: idp.Options{ IsCreationAllowed: true, IsLinkingAllowed: true, @@ -805,6 +814,7 @@ func TestCommandSide_AddInstanceGenericOIDCIDP(t *testing.T) { }, nil, false, + true, idp.Options{}, ), ), @@ -819,6 +829,7 @@ func TestCommandSide_AddInstanceGenericOIDCIDP(t *testing.T) { Issuer: "issuer", ClientID: "clientID", ClientSecret: "clientSecret", + UsePKCE: true, }, }, res: res{ @@ -845,6 +856,7 @@ func TestCommandSide_AddInstanceGenericOIDCIDP(t *testing.T) { }, []string{openid.ScopeOpenID}, true, + true, idp.Options{ IsCreationAllowed: true, IsLinkingAllowed: true, @@ -866,6 +878,7 @@ func TestCommandSide_AddInstanceGenericOIDCIDP(t *testing.T) { ClientSecret: "clientSecret", Scopes: []string{openid.ScopeOpenID}, IsIDTokenMapping: true, + UsePKCE: true, IDPOptions: idp.Options{ IsCreationAllowed: true, IsLinkingAllowed: true, @@ -1029,6 +1042,7 @@ func TestCommandSide_UpdateInstanceGenericOIDCIDP(t *testing.T) { }, nil, false, + false, idp.Options{}, )), ), @@ -1066,6 +1080,7 @@ func TestCommandSide_UpdateInstanceGenericOIDCIDP(t *testing.T) { }, nil, false, + false, idp.Options{}, )), ), @@ -1086,6 +1101,7 @@ func TestCommandSide_UpdateInstanceGenericOIDCIDP(t *testing.T) { }), idp.ChangeOIDCScopes([]string{"openid", "profile"}), idp.ChangeOIDCIsIDTokenMapping(true), + idp.ChangeOIDCUsePKCE(true), idp.ChangeOIDCOptions(idp.OptionChanges{ IsCreationAllowed: &t, IsLinkingAllowed: &t, @@ -1110,6 +1126,7 @@ func TestCommandSide_UpdateInstanceGenericOIDCIDP(t *testing.T) { ClientSecret: "newSecret", Scopes: []string{"openid", "profile"}, IsIDTokenMapping: true, + UsePKCE: true, IDPOptions: idp.Options{ IsCreationAllowed: true, IsLinkingAllowed: true, @@ -1253,6 +1270,7 @@ func TestCommandSide_MigrateInstanceGenericOIDCToAzureADProvider(t *testing.T) { }, nil, false, + false, idp.Options{}, )), ), @@ -1311,6 +1329,7 @@ func TestCommandSide_MigrateInstanceGenericOIDCToAzureADProvider(t *testing.T) { }, nil, false, + false, idp.Options{}, )), ), @@ -1475,6 +1494,7 @@ func TestCommandSide_MigrateInstanceOIDCToGoogleIDP(t *testing.T) { }, nil, false, + false, idp.Options{}, )), ), @@ -1527,6 +1547,7 @@ func TestCommandSide_MigrateInstanceOIDCToGoogleIDP(t *testing.T) { }, nil, false, + false, idp.Options{}, )), ), diff --git a/internal/command/org_idp.go b/internal/command/org_idp.go index 21d871133a..d24b0f7840 100644 --- a/internal/command/org_idp.go +++ b/internal/command/org_idp.go @@ -628,6 +628,7 @@ func (c *Commands) prepareAddOrgOAuthProvider(a *org.Aggregate, writeModel *OrgO provider.UserEndpoint, provider.IDAttribute, provider.Scopes, + provider.UsePKCE, provider.IDPOptions, ), }, nil @@ -683,6 +684,7 @@ func (c *Commands) prepareUpdateOrgOAuthProvider(a *org.Aggregate, writeModel *O provider.UserEndpoint, provider.IDAttribute, provider.Scopes, + provider.UsePKCE, provider.IDPOptions, ) if err != nil || event == nil { @@ -731,6 +733,7 @@ func (c *Commands) prepareAddOrgOIDCProvider(a *org.Aggregate, writeModel *OrgOI secret, provider.Scopes, provider.IsIDTokenMapping, + provider.UsePKCE, provider.IDPOptions, ), }, nil @@ -775,6 +778,7 @@ func (c *Commands) prepareUpdateOrgOIDCProvider(a *org.Aggregate, writeModel *Or c.idpConfigEncryption, provider.Scopes, provider.IsIDTokenMapping, + provider.UsePKCE, provider.IDPOptions, ) if err != nil || event == nil { diff --git a/internal/command/org_idp_model.go b/internal/command/org_idp_model.go index 634eb42eaf..3baea11495 100644 --- a/internal/command/org_idp_model.go +++ b/internal/command/org_idp_model.go @@ -70,6 +70,7 @@ func (wm *OrgOAuthIDPWriteModel) NewChangedEvent( userEndpoint, idAttribute string, scopes []string, + usePKCE bool, options idp.Options, ) (*org.OAuthIDPChangedEvent, error) { @@ -83,6 +84,7 @@ func (wm *OrgOAuthIDPWriteModel) NewChangedEvent( userEndpoint, idAttribute, scopes, + usePKCE, options, ) if err != nil || len(changes) == 0 { @@ -176,7 +178,7 @@ func (wm *OrgOIDCIDPWriteModel) NewChangedEvent( clientSecretString string, secretCrypto crypto.EncryptionAlgorithm, scopes []string, - idTokenMapping bool, + idTokenMapping, usePKCE bool, options idp.Options, ) (*org.OIDCIDPChangedEvent, error) { @@ -188,6 +190,7 @@ func (wm *OrgOIDCIDPWriteModel) NewChangedEvent( secretCrypto, scopes, idTokenMapping, + usePKCE, options, ) if err != nil || len(changes) == 0 { diff --git a/internal/command/org_idp_test.go b/internal/command/org_idp_test.go index e2fd68fa90..25115f71fe 100644 --- a/internal/command/org_idp_test.go +++ b/internal/command/org_idp_test.go @@ -210,6 +210,7 @@ func TestCommandSide_AddOrgGenericOAuthProvider(t *testing.T) { "user", "idAttribute", nil, + false, idp.Options{}, ), ), @@ -256,6 +257,7 @@ func TestCommandSide_AddOrgGenericOAuthProvider(t *testing.T) { "user", "idAttribute", []string{"user"}, + true, idp.Options{ IsCreationAllowed: true, IsLinkingAllowed: true, @@ -280,6 +282,7 @@ func TestCommandSide_AddOrgGenericOAuthProvider(t *testing.T) { UserEndpoint: "user", Scopes: []string{"user"}, IDAttribute: "idAttribute", + UsePKCE: true, IDPOptions: idp.Options{ IsCreationAllowed: true, IsLinkingAllowed: true, @@ -520,6 +523,7 @@ func TestCommandSide_UpdateOrgGenericOAuthIDP(t *testing.T) { "user", "idAttribute", nil, + false, idp.Options{}, )), ), @@ -536,6 +540,7 @@ func TestCommandSide_UpdateOrgGenericOAuthIDP(t *testing.T) { TokenEndpoint: "token", UserEndpoint: "user", IDAttribute: "idAttribute", + UsePKCE: false, }, }, res: res{ @@ -563,6 +568,7 @@ func TestCommandSide_UpdateOrgGenericOAuthIDP(t *testing.T) { "user", "idAttribute", nil, + false, idp.Options{}, )), ), @@ -585,6 +591,7 @@ func TestCommandSide_UpdateOrgGenericOAuthIDP(t *testing.T) { idp.ChangeOAuthUserEndpoint("new user"), idp.ChangeOAuthScopes([]string{"openid", "profile"}), idp.ChangeOAuthIDAttribute("newAttribute"), + idp.ChangeOAuthUsePKCE(true), idp.ChangeOAuthOptions(idp.OptionChanges{ IsCreationAllowed: &t, IsLinkingAllowed: &t, @@ -612,6 +619,7 @@ func TestCommandSide_UpdateOrgGenericOAuthIDP(t *testing.T) { UserEndpoint: "new user", Scopes: []string{"openid", "profile"}, IDAttribute: "newAttribute", + UsePKCE: true, IDPOptions: idp.Options{ IsCreationAllowed: true, IsLinkingAllowed: true, @@ -763,6 +771,7 @@ func TestCommandSide_AddOrgGenericOIDCIDP(t *testing.T) { }, nil, false, + false, idp.Options{}, ), ), @@ -804,6 +813,7 @@ func TestCommandSide_AddOrgGenericOIDCIDP(t *testing.T) { }, []string{openid.ScopeOpenID}, true, + true, idp.Options{ IsCreationAllowed: true, IsLinkingAllowed: true, @@ -826,6 +836,7 @@ func TestCommandSide_AddOrgGenericOIDCIDP(t *testing.T) { ClientSecret: "clientSecret", Scopes: []string{openid.ScopeOpenID}, IsIDTokenMapping: true, + UsePKCE: true, IDPOptions: idp.Options{ IsCreationAllowed: true, IsLinkingAllowed: true, @@ -995,6 +1006,7 @@ func TestCommandSide_UpdateOrgGenericOIDCIDP(t *testing.T) { }, nil, false, + false, idp.Options{}, )), ), @@ -1033,6 +1045,7 @@ func TestCommandSide_UpdateOrgGenericOIDCIDP(t *testing.T) { }, nil, false, + false, idp.Options{}, )), ), @@ -1053,6 +1066,7 @@ func TestCommandSide_UpdateOrgGenericOIDCIDP(t *testing.T) { }), idp.ChangeOIDCScopes([]string{"openid", "profile"}), idp.ChangeOIDCIsIDTokenMapping(true), + idp.ChangeOIDCUsePKCE(true), idp.ChangeOIDCOptions(idp.OptionChanges{ IsCreationAllowed: &t, IsLinkingAllowed: &t, @@ -1078,6 +1092,7 @@ func TestCommandSide_UpdateOrgGenericOIDCIDP(t *testing.T) { ClientSecret: "newSecret", Scopes: []string{"openid", "profile"}, IsIDTokenMapping: true, + UsePKCE: true, IDPOptions: idp.Options{ IsCreationAllowed: true, IsLinkingAllowed: true, @@ -1225,6 +1240,7 @@ func TestCommandSide_MigrateOrgGenericOIDCToAzureADProvider(t *testing.T) { }, nil, false, + false, idp.Options{}, )), ), @@ -1284,6 +1300,7 @@ func TestCommandSide_MigrateOrgGenericOIDCToAzureADProvider(t *testing.T) { }, nil, false, + false, idp.Options{}, )), ), @@ -1452,6 +1469,7 @@ func TestCommandSide_MigrateOrgOIDCToGoogleIDP(t *testing.T) { }, nil, false, + false, idp.Options{}, )), ), @@ -1505,6 +1523,7 @@ func TestCommandSide_MigrateOrgOIDCToGoogleIDP(t *testing.T) { }, nil, false, + false, idp.Options{}, )), ), diff --git a/internal/command/session_test.go b/internal/command/session_test.go index 5fba985148..60027d3a05 100644 --- a/internal/command/session_test.go +++ b/internal/command/session_test.go @@ -837,6 +837,7 @@ func TestCommands_updateSession(t *testing.T) { nil, nil, "idpID", + nil, ), ), eventFromEventPusher( diff --git a/internal/domain/auth_request.go b/internal/domain/auth_request.go index 85ec340f67..c880c5159e 100644 --- a/internal/domain/auth_request.go +++ b/internal/domain/auth_request.go @@ -43,6 +43,7 @@ type AuthRequest struct { ApplicationResourceOwner string PrivateLabelingSetting PrivateLabelingSetting SelectedIDPConfigID string + SelectedIDPConfigArgs map[string]any LinkingUsers []*ExternalUser PossibleSteps []NextStep `json:"-"` PasswordVerified bool diff --git a/internal/idp/providers/apple/session.go b/internal/idp/providers/apple/session.go index 5e9143f050..eee68fa2a5 100644 --- a/internal/idp/providers/apple/session.go +++ b/internal/idp/providers/apple/session.go @@ -17,6 +17,10 @@ type Session struct { UserFormValue string } +func NewSession(provider *Provider, code, userFormValue string) *Session { + return &Session{Session: oidc.NewSession(provider.Provider, code, nil), UserFormValue: userFormValue} +} + type userFormValue struct { Name userNamesFormValue `json:"name,omitempty" schema:"name"` } diff --git a/internal/idp/providers/azuread/session.go b/internal/idp/providers/azuread/session.go index a9d8df2e8c..4b0a6fb844 100644 --- a/internal/idp/providers/azuread/session.go +++ b/internal/idp/providers/azuread/session.go @@ -20,6 +20,10 @@ type Session struct { OAuthSession *oauth.Session } +func NewSession(provider *Provider, code string) *Session { + return &Session{Provider: provider, Code: code} +} + // GetAuth implements the [idp.Provider] interface by calling the wrapped [oauth.Session]. func (s *Session) GetAuth(ctx context.Context) (content string, redirect bool) { return s.oauth().GetAuth(ctx) @@ -39,6 +43,11 @@ func (s *Session) RetrievePreviousID() (string, error) { return userinfo.Subject, nil } +// PersistentParameters implements the [idp.Session] interface. +func (s *Session) PersistentParameters() map[string]any { + return nil +} + // FetchUser implements the [idp.Session] interface. // It will execute an OAuth 2.0 code exchange if needed to retrieve the access token, // call the specified userEndpoint and map the received information into an [idp.User]. diff --git a/internal/idp/providers/jwt/session.go b/internal/idp/providers/jwt/session.go index 54fcc039eb..6df08a6998 100644 --- a/internal/idp/providers/jwt/session.go +++ b/internal/idp/providers/jwt/session.go @@ -30,11 +30,20 @@ type Session struct { Tokens *oidc.Tokens[*oidc.IDTokenClaims] } +func NewSession(provider *Provider, tokens *oidc.Tokens[*oidc.IDTokenClaims]) *Session { + return &Session{Provider: provider, Tokens: tokens} +} + // GetAuth implements the [idp.Session] interface. func (s *Session) GetAuth(ctx context.Context) (string, bool) { return idp.Redirect(s.AuthURL) } +// PersistentParameters implements the [idp.Session] interface. +func (s *Session) PersistentParameters() map[string]any { + return nil +} + // FetchUser implements the [idp.Session] interface. // It will map the received idToken into an [idp.User]. func (s *Session) FetchUser(ctx context.Context) (user idp.User, err error) { diff --git a/internal/idp/providers/ldap/session.go b/internal/idp/providers/ldap/session.go index 4984ad3531..0a6a87ba3d 100644 --- a/internal/idp/providers/ldap/session.go +++ b/internal/idp/providers/ldap/session.go @@ -34,11 +34,21 @@ type Session struct { Entry *ldap.Entry } +func NewSession(provider *Provider, username, password string) *Session { + return &Session{Provider: provider, User: username, Password: password} +} + // GetAuth implements the [idp.Session] interface. func (s *Session) GetAuth(ctx context.Context) (string, bool) { return idp.Redirect(s.loginUrl) } +// PersistentParameters implements the [idp.Session] interface. +func (s *Session) PersistentParameters() map[string]any { + return nil +} + +// FetchUser implements the [idp.Session] interface. func (s *Session) FetchUser(_ context.Context) (_ idp.User, err error) { var user *ldap.Entry for _, server := range s.Provider.servers { diff --git a/internal/idp/providers/oauth/oauth2.go b/internal/idp/providers/oauth/oauth2.go index 910ceb1b9c..e9c627509a 100644 --- a/internal/idp/providers/oauth/oauth2.go +++ b/internal/idp/providers/oauth/oauth2.go @@ -23,6 +23,7 @@ type Provider struct { isCreationAllowed bool isAutoCreation bool isAutoUpdate bool + generateVerifier func() string } type ProviderOpts func(provider *Provider) @@ -66,9 +67,10 @@ func WithRelyingPartyOption(option rp.Option) ProviderOpts { // New creates a generic OAuth 2.0 provider func New(config *oauth2.Config, name, userEndpoint string, userMapper func() idp.User, options ...ProviderOpts) (provider *Provider, err error) { provider = &Provider{ - name: name, - userEndpoint: userEndpoint, - userMapper: userMapper, + name: name, + userEndpoint: userEndpoint, + userMapper: userMapper, + generateVerifier: oauth2.GenerateVerifier, } for _, option := range options { option(provider) @@ -99,8 +101,15 @@ func (p *Provider) BeginAuth(ctx context.Context, state string, params ...idp.Pa if !loginHintSet { opts = append(opts, rp.WithPrompt(oidc.PromptSelectAccount)) } + + var codeVerifier string + if p.RelyingParty.IsPKCE() { + codeVerifier = p.generateVerifier() + opts = append(opts, rp.WithCodeChallenge(oidc.NewSHACodeChallenge(codeVerifier))) + } + url := rp.AuthURL(state, p.RelyingParty, opts...) - return &Session{AuthURL: url, Provider: p}, nil + return &Session{AuthURL: url, Provider: p, CodeVerifier: codeVerifier}, nil } func loginHint(hint string) rp.AuthURLOpt { diff --git a/internal/idp/providers/oauth/oauth2_test.go b/internal/idp/providers/oauth/oauth2_test.go index 814a7ac9c2..984315ac1f 100644 --- a/internal/idp/providers/oauth/oauth2_test.go +++ b/internal/idp/providers/oauth/oauth2_test.go @@ -18,6 +18,7 @@ func TestProvider_BeginAuth(t *testing.T) { name string userEndpoint string userMapper func() idp.User + options []ProviderOpts } tests := []struct { name string @@ -25,7 +26,7 @@ func TestProvider_BeginAuth(t *testing.T) { want idp.Session }{ { - name: "successful auth", + name: "successful auth without PKCE", fields: fields{ config: &oauth2.Config{ ClientID: "clientID", @@ -40,14 +41,40 @@ func TestProvider_BeginAuth(t *testing.T) { }, want: &Session{AuthURL: "https://oauth2.com/authorize?client_id=clientID&prompt=select_account&redirect_uri=redirectURI&response_type=code&scope=user&state=testState"}, }, + { + name: "successful auth with PKCE", + fields: fields{ + config: &oauth2.Config{ + ClientID: "clientID", + ClientSecret: "clientSecret", + Endpoint: oauth2.Endpoint{ + AuthURL: "https://oauth2.com/authorize", + TokenURL: "https://oauth2.com/token", + }, + RedirectURL: "redirectURI", + Scopes: []string{"user"}, + }, + options: []ProviderOpts{ + WithLinkingAllowed(), + WithCreationAllowed(), + WithAutoCreation(), + WithAutoUpdate(), + WithRelyingPartyOption(rp.WithPKCE(nil)), + }, + }, + want: &Session{AuthURL: "https://oauth2.com/authorize?client_id=clientID&code_challenge=2ZoH_a01aprzLkwVbjlPsBo4m8mJ_zOKkaDqYM7Oh5w&code_challenge_method=S256&prompt=select_account&redirect_uri=redirectURI&response_type=code&scope=user&state=testState"}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { a := assert.New(t) r := require.New(t) - provider, err := New(tt.fields.config, tt.fields.name, tt.fields.userEndpoint, tt.fields.userMapper) + provider, err := New(tt.fields.config, tt.fields.name, tt.fields.userEndpoint, tt.fields.userMapper, tt.fields.options...) r.NoError(err) + provider.generateVerifier = func() string { + return "pkceOAuthVerifier" + } ctx := context.Background() session, err := provider.BeginAuth(ctx, "testState") diff --git a/internal/idp/providers/oauth/session.go b/internal/idp/providers/oauth/session.go index 065ad3b213..aca22234a2 100644 --- a/internal/idp/providers/oauth/session.go +++ b/internal/idp/providers/oauth/session.go @@ -14,22 +14,40 @@ import ( var ErrCodeMissing = errors.New("no auth code provided") +const ( + CodeVerifier = "codeVerifier" +) + var _ idp.Session = (*Session)(nil) // Session is the [idp.Session] implementation for the OAuth2.0 provider. type Session struct { - AuthURL string - Code string - Tokens *oidc.Tokens[*oidc.IDTokenClaims] + AuthURL string + CodeVerifier string + Code string + Tokens *oidc.Tokens[*oidc.IDTokenClaims] Provider *Provider } +func NewSession(provider *Provider, code string, idpArguments map[string]any) *Session { + verifier, _ := idpArguments[CodeVerifier].(string) + return &Session{Provider: provider, Code: code, CodeVerifier: verifier} +} + // GetAuth implements the [idp.Session] interface. func (s *Session) GetAuth(ctx context.Context) (string, bool) { return idp.Redirect(s.AuthURL) } +// PersistentParameters implements the [idp.Session] interface. +func (s *Session) PersistentParameters() map[string]any { + if s.CodeVerifier == "" { + return nil + } + return map[string]any{CodeVerifier: s.CodeVerifier} +} + // FetchUser implements the [idp.Session] interface. // It will execute an OAuth 2.0 code exchange if needed to retrieve the access token, // call the specified userEndpoint and map the received information into an [idp.User]. @@ -55,7 +73,11 @@ func (s *Session) authorize(ctx context.Context) (err error) { if s.Code == "" { return ErrCodeMissing } - s.Tokens, err = rp.CodeExchange[*oidc.IDTokenClaims](ctx, s.Code, s.Provider.RelyingParty) + var opts []rp.CodeExchangeOpt + if s.CodeVerifier != "" { + opts = append(opts, rp.WithCodeVerifier(s.CodeVerifier)) + } + s.Tokens, err = rp.CodeExchange[*oidc.IDTokenClaims](ctx, s.Code, s.Provider.RelyingParty, opts...) return err } diff --git a/internal/idp/providers/oidc/oidc.go b/internal/idp/providers/oidc/oidc.go index f122230c3a..cd3ac764fd 100644 --- a/internal/idp/providers/oidc/oidc.go +++ b/internal/idp/providers/oidc/oidc.go @@ -24,6 +24,7 @@ type Provider struct { useIDToken bool userInfoMapper func(info *oidc.UserInfo) idp.User authOptions []func(bool) rp.AuthURLOpt + generateVerifier func() string } type ProviderOpts func(provider *Provider) @@ -102,8 +103,9 @@ var DefaultMapper UserInfoMapper = func(info *oidc.UserInfo) idp.User { // New creates a generic OIDC provider func New(name, issuer, clientID, clientSecret, redirectURI string, scopes []string, userInfoMapper UserInfoMapper, options ...ProviderOpts) (provider *Provider, err error) { provider = &Provider{ - name: name, - userInfoMapper: userInfoMapper, + name: name, + userInfoMapper: userInfoMapper, + generateVerifier: oauth2.GenerateVerifier, } for _, option := range options { option(provider) @@ -150,8 +152,15 @@ func (p *Provider) BeginAuth(ctx context.Context, state string, params ...idp.Pa opts = append(opts, opt) } } + + var codeVerifier string + if p.RelyingParty.IsPKCE() { + codeVerifier = p.generateVerifier() + opts = append(opts, rp.WithCodeChallenge(oidc.NewSHACodeChallenge(codeVerifier))) + } + url := rp.AuthURL(state, p.RelyingParty, opts...) - return &Session{AuthURL: url, Provider: p}, nil + return &Session{AuthURL: url, Provider: p, CodeVerifier: codeVerifier}, nil } func loginHint(hint string) rp.AuthURLOpt { diff --git a/internal/idp/providers/oidc/oidc_test.go b/internal/idp/providers/oidc/oidc_test.go index d510bf15c2..a46f09f13f 100644 --- a/internal/idp/providers/oidc/oidc_test.go +++ b/internal/idp/providers/oidc/oidc_test.go @@ -31,7 +31,7 @@ func TestProvider_BeginAuth(t *testing.T) { want idp.Session }{ { - name: "successful auth", + name: "successful auth without PKCE", fields: fields{ name: "oidc", issuer: "https://issuer.com", @@ -55,6 +55,31 @@ func TestProvider_BeginAuth(t *testing.T) { }, want: &Session{AuthURL: "https://issuer.com/authorize?client_id=clientID&prompt=select_account&redirect_uri=redirectURI&response_type=code&scope=openid&state=testState"}, }, + { + name: "successful auth with PKCE", + fields: fields{ + name: "oidc", + issuer: "https://issuer.com", + clientID: "clientID", + clientSecret: "clientSecret", + redirectURI: "redirectURI", + scopes: []string{"openid"}, + userMapper: DefaultMapper, + httpMock: func(issuer string) { + gock.New(issuer). + Get(oidc.DiscoveryEndpoint). + Reply(200). + JSON(&oidc.DiscoveryConfiguration{ + Issuer: issuer, + AuthorizationEndpoint: issuer + "/authorize", + TokenEndpoint: issuer + "/token", + UserinfoEndpoint: issuer + "/userinfo", + }) + }, + opts: []ProviderOpts{WithSelectAccount(), WithRelyingPartyOption(rp.WithPKCE(nil))}, + }, + want: &Session{AuthURL: "https://issuer.com/authorize?client_id=clientID&code_challenge=2ZoH_a01aprzLkwVbjlPsBo4m8mJ_zOKkaDqYM7Oh5w&code_challenge_method=S256&prompt=select_account&redirect_uri=redirectURI&response_type=code&scope=openid&state=testState"}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -65,6 +90,9 @@ func TestProvider_BeginAuth(t *testing.T) { provider, err := New(tt.fields.name, tt.fields.issuer, tt.fields.clientID, tt.fields.clientSecret, tt.fields.redirectURI, tt.fields.scopes, tt.fields.userMapper, tt.fields.opts...) r.NoError(err) + provider.generateVerifier = func() string { + return "pkceOAuthVerifier" + } ctx := context.Background() session, err := provider.BeginAuth(ctx, "testState") diff --git a/internal/idp/providers/oidc/session.go b/internal/idp/providers/oidc/session.go index bd6303f2e5..b17a3b0a0b 100644 --- a/internal/idp/providers/oidc/session.go +++ b/internal/idp/providers/oidc/session.go @@ -10,6 +10,7 @@ import ( "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/idp" + "github.com/zitadel/zitadel/internal/idp/providers/oauth" ) var ErrCodeMissing = errors.New("no auth code provided") @@ -18,10 +19,16 @@ var _ idp.Session = (*Session)(nil) // Session is the [idp.Session] implementation for the OIDC provider. type Session struct { - Provider *Provider - AuthURL string - Code string - Tokens *oidc.Tokens[*oidc.IDTokenClaims] + Provider *Provider + AuthURL string + CodeVerifier string + Code string + Tokens *oidc.Tokens[*oidc.IDTokenClaims] +} + +func NewSession(provider *Provider, code string, idpArguments map[string]any) *Session { + verifier, _ := idpArguments[oauth.CodeVerifier].(string) + return &Session{Provider: provider, Code: code, CodeVerifier: verifier} } // GetAuth implements the [idp.Session] interface. @@ -29,6 +36,14 @@ func (s *Session) GetAuth(ctx context.Context) (string, bool) { return idp.Redirect(s.AuthURL) } +// PersistentParameters implements the [idp.Session] interface. +func (s *Session) PersistentParameters() map[string]any { + if s.CodeVerifier == "" { + return nil + } + return map[string]any{oauth.CodeVerifier: s.CodeVerifier} +} + // FetchUser implements the [idp.Session] interface. // It will execute an OIDC code exchange if needed to retrieve the tokens, // call the userinfo endpoint and map the received information into an [idp.User]. @@ -61,7 +76,11 @@ func (s *Session) Authorize(ctx context.Context) (err error) { if s.Code == "" { return ErrCodeMissing } - s.Tokens, err = rp.CodeExchange[*oidc.IDTokenClaims](ctx, s.Code, s.Provider.RelyingParty) + var opts []rp.CodeExchangeOpt + if s.CodeVerifier != "" { + opts = append(opts, rp.WithCodeVerifier(s.CodeVerifier)) + } + s.Tokens, err = rp.CodeExchange[*oidc.IDTokenClaims](ctx, s.Code, s.Provider.RelyingParty, opts...) return err } diff --git a/internal/idp/providers/saml/session.go b/internal/idp/providers/saml/session.go index 49a04e49cb..b0748d33a3 100644 --- a/internal/idp/providers/saml/session.go +++ b/internal/idp/providers/saml/session.go @@ -60,6 +60,11 @@ func (s *Session) GetAuth(ctx context.Context) (string, bool) { return idp.Form(resp.content.String()) } +// PersistentParameters implements the [idp.Session] interface. +func (s *Session) PersistentParameters() map[string]any { + return nil +} + // FetchUser implements the [idp.Session] interface. func (s *Session) FetchUser(ctx context.Context) (user idp.User, err error) { if s.RequestID == "" || s.Request == nil { diff --git a/internal/idp/session.go b/internal/idp/session.go index 6d6519a54c..ab54bcabaa 100644 --- a/internal/idp/session.go +++ b/internal/idp/session.go @@ -7,6 +7,7 @@ import ( // Session is the minimal implementation for a session of a 3rd party authentication [Provider] type Session interface { GetAuth(ctx context.Context) (content string, redirect bool) + PersistentParameters() map[string]any FetchUser(ctx context.Context) (User, error) } diff --git a/internal/integration/sink/server.go b/internal/integration/sink/server.go index eea7c893b6..aee40cad02 100644 --- a/internal/integration/sink/server.go +++ b/internal/integration/sink/server.go @@ -327,7 +327,7 @@ func successfulIntentHandler(cmd *command.Commands, createIntent func(ctx contex } func createIntent(ctx context.Context, cmd *command.Commands, instanceID, idpID string) (string, error) { - writeModel, _, err := cmd.CreateIntent(ctx, idpID, "https://example.com/success", "https://example.com/failure", instanceID) + writeModel, _, err := cmd.CreateIntent(ctx, "", idpID, "https://example.com/success", "https://example.com/failure", instanceID, nil) if err != nil { return "", err } diff --git a/internal/query/idp_template.go b/internal/query/idp_template.go index d75e040ada..a63cb6f485 100644 --- a/internal/query/idp_template.go +++ b/internal/query/idp_template.go @@ -64,6 +64,7 @@ type OAuthIDPTemplate struct { UserEndpoint string Scopes database.TextArray[string] IDAttribute string + UsePKCE bool } type OIDCIDPTemplate struct { @@ -73,6 +74,7 @@ type OIDCIDPTemplate struct { Issuer string Scopes database.TextArray[string] IsIDTokenMapping bool + UsePKCE bool } type JWTIDPTemplate struct { @@ -278,6 +280,10 @@ var ( name: projection.OAuthIDAttributeCol, table: oauthIdpTemplateTable, } + OAuthUsePKCECol = Column{ + name: projection.OAuthUsePKCECol, + table: oauthIdpTemplateTable, + } ) var ( @@ -313,6 +319,10 @@ var ( name: projection.OIDCIDTokenMappingCol, table: oidcIdpTemplateTable, } + OIDCUsePKCECol = Column{ + name: projection.OIDCUsePKCECol, + table: oidcIdpTemplateTable, + } ) var ( @@ -879,6 +889,7 @@ func prepareIDPTemplateByIDQuery(ctx context.Context, db prepareDatabase) (sq.Se OAuthUserEndpointCol.identifier(), OAuthScopesCol.identifier(), OAuthIDAttributeCol.identifier(), + OAuthUsePKCECol.identifier(), // oidc OIDCIDCol.identifier(), OIDCIssuerCol.identifier(), @@ -886,6 +897,7 @@ func prepareIDPTemplateByIDQuery(ctx context.Context, db prepareDatabase) (sq.Se OIDCClientSecretCol.identifier(), OIDCScopesCol.identifier(), OIDCIDTokenMappingCol.identifier(), + OIDCUsePKCECol.identifier(), // jwt JWTIDCol.identifier(), JWTIssuerCol.identifier(), @@ -996,6 +1008,7 @@ func prepareIDPTemplateByIDQuery(ctx context.Context, db prepareDatabase) (sq.Se oauthUserEndpoint := sql.NullString{} oauthScopes := database.TextArray[string]{} oauthIDAttribute := sql.NullString{} + oauthUserPKCE := sql.NullBool{} oidcID := sql.NullString{} oidcIssuer := sql.NullString{} @@ -1003,6 +1016,7 @@ func prepareIDPTemplateByIDQuery(ctx context.Context, db prepareDatabase) (sq.Se oidcClientSecret := new(crypto.CryptoValue) oidcScopes := database.TextArray[string]{} oidcIDTokenMapping := sql.NullBool{} + oidcUserPKCE := sql.NullBool{} jwtID := sql.NullString{} jwtIssuer := sql.NullString{} @@ -1111,6 +1125,7 @@ func prepareIDPTemplateByIDQuery(ctx context.Context, db prepareDatabase) (sq.Se &oauthUserEndpoint, &oauthScopes, &oauthIDAttribute, + &oauthUserPKCE, // oidc &oidcID, &oidcIssuer, @@ -1118,6 +1133,7 @@ func prepareIDPTemplateByIDQuery(ctx context.Context, db prepareDatabase) (sq.Se &oidcClientSecret, &oidcScopes, &oidcIDTokenMapping, + &oidcUserPKCE, // jwt &jwtID, &jwtIssuer, @@ -1221,6 +1237,7 @@ func prepareIDPTemplateByIDQuery(ctx context.Context, db prepareDatabase) (sq.Se UserEndpoint: oauthUserEndpoint.String, Scopes: oauthScopes, IDAttribute: oauthIDAttribute.String, + UsePKCE: oauthUserPKCE.Bool, } } if oidcID.Valid { @@ -1231,6 +1248,7 @@ func prepareIDPTemplateByIDQuery(ctx context.Context, db prepareDatabase) (sq.Se Issuer: oidcIssuer.String, Scopes: oidcScopes, IsIDTokenMapping: oidcIDTokenMapping.Bool, + UsePKCE: oidcUserPKCE.Bool, } } if jwtID.Valid { @@ -1378,6 +1396,7 @@ func prepareIDPTemplatesQuery(ctx context.Context, db prepareDatabase) (sq.Selec OAuthUserEndpointCol.identifier(), OAuthScopesCol.identifier(), OAuthIDAttributeCol.identifier(), + OAuthUsePKCECol.identifier(), // oidc OIDCIDCol.identifier(), OIDCIssuerCol.identifier(), @@ -1385,6 +1404,7 @@ func prepareIDPTemplatesQuery(ctx context.Context, db prepareDatabase) (sq.Selec OIDCClientSecretCol.identifier(), OIDCScopesCol.identifier(), OIDCIDTokenMappingCol.identifier(), + OIDCUsePKCECol.identifier(), // jwt JWTIDCol.identifier(), JWTIssuerCol.identifier(), @@ -1500,6 +1520,7 @@ func prepareIDPTemplatesQuery(ctx context.Context, db prepareDatabase) (sq.Selec oauthUserEndpoint := sql.NullString{} oauthScopes := database.TextArray[string]{} oauthIDAttribute := sql.NullString{} + oauthUserPKCE := sql.NullBool{} oidcID := sql.NullString{} oidcIssuer := sql.NullString{} @@ -1507,6 +1528,7 @@ func prepareIDPTemplatesQuery(ctx context.Context, db prepareDatabase) (sq.Selec oidcClientSecret := new(crypto.CryptoValue) oidcScopes := database.TextArray[string]{} oidcIDTokenMapping := sql.NullBool{} + oidcUserPKCE := sql.NullBool{} jwtID := sql.NullString{} jwtIssuer := sql.NullString{} @@ -1615,6 +1637,7 @@ func prepareIDPTemplatesQuery(ctx context.Context, db prepareDatabase) (sq.Selec &oauthUserEndpoint, &oauthScopes, &oauthIDAttribute, + &oauthUserPKCE, // oidc &oidcID, &oidcIssuer, @@ -1622,6 +1645,7 @@ func prepareIDPTemplatesQuery(ctx context.Context, db prepareDatabase) (sq.Selec &oidcClientSecret, &oidcScopes, &oidcIDTokenMapping, + &oidcUserPKCE, // jwt &jwtID, &jwtIssuer, @@ -1724,6 +1748,7 @@ func prepareIDPTemplatesQuery(ctx context.Context, db prepareDatabase) (sq.Selec UserEndpoint: oauthUserEndpoint.String, Scopes: oauthScopes, IDAttribute: oauthIDAttribute.String, + UsePKCE: oauthUserPKCE.Bool, } } if oidcID.Valid { @@ -1734,6 +1759,7 @@ func prepareIDPTemplatesQuery(ctx context.Context, db prepareDatabase) (sq.Selec Issuer: oidcIssuer.String, Scopes: oidcScopes, IsIDTokenMapping: oidcIDTokenMapping.Bool, + UsePKCE: oidcUserPKCE.Bool, } } if jwtID.Valid { diff --git a/internal/query/idp_template_test.go b/internal/query/idp_template_test.go index 1c2310fee6..bee19e492a 100644 --- a/internal/query/idp_template_test.go +++ b/internal/query/idp_template_test.go @@ -39,6 +39,7 @@ var ( ` projections.idp_templates6_oauth2.user_endpoint,` + ` projections.idp_templates6_oauth2.scopes,` + ` projections.idp_templates6_oauth2.id_attribute,` + + ` projections.idp_templates6_oauth2.use_pkce,` + // oidc ` projections.idp_templates6_oidc.idp_id,` + ` projections.idp_templates6_oidc.issuer,` + @@ -46,6 +47,7 @@ var ( ` projections.idp_templates6_oidc.client_secret,` + ` projections.idp_templates6_oidc.scopes,` + ` projections.idp_templates6_oidc.id_token_mapping,` + + ` projections.idp_templates6_oidc.use_pkce,` + // jwt ` projections.idp_templates6_jwt.idp_id,` + ` projections.idp_templates6_jwt.issuer,` + @@ -167,6 +169,7 @@ var ( "user_endpoint", "scopes", "id_attribute", + "use_pkce", // oidc config "id_id", "issuer", @@ -174,6 +177,7 @@ var ( "client_secret", "scopes", "id_token_mapping", + "use_pkce", // jwt "idp_id", "issuer", @@ -281,6 +285,7 @@ var ( ` projections.idp_templates6_oauth2.user_endpoint,` + ` projections.idp_templates6_oauth2.scopes,` + ` projections.idp_templates6_oauth2.id_attribute,` + + ` projections.idp_templates6_oauth2.use_pkce,` + // oidc ` projections.idp_templates6_oidc.idp_id,` + ` projections.idp_templates6_oidc.issuer,` + @@ -288,6 +293,7 @@ var ( ` projections.idp_templates6_oidc.client_secret,` + ` projections.idp_templates6_oidc.scopes,` + ` projections.idp_templates6_oidc.id_token_mapping,` + + ` projections.idp_templates6_oidc.use_pkce,` + // jwt ` projections.idp_templates6_jwt.idp_id,` + ` projections.idp_templates6_jwt.issuer,` + @@ -410,6 +416,7 @@ var ( "user_endpoint", "scopes", "id_attribute", + "use_pkce", // oidc config "id_id", "issuer", @@ -417,6 +424,7 @@ var ( "client_secret", "scopes", "id_token_mapping", + "use_pkce", // jwt "idp_id", "issuer", @@ -564,6 +572,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { "user", database.TextArray[string]{"profile"}, "id-attribute", + true, // oidc nil, nil, @@ -571,6 +580,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -681,6 +691,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { UserEndpoint: "user", Scopes: []string{"profile"}, IDAttribute: "id-attribute", + UsePKCE: true, }, }, }, @@ -715,6 +726,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc "idp-id", "issuer", @@ -722,6 +734,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, database.TextArray[string]{"profile"}, true, + true, // jwt nil, nil, @@ -830,6 +843,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { ClientSecret: nil, Scopes: []string{"profile"}, IsIDTokenMapping: true, + UsePKCE: true, }, }, }, @@ -864,6 +878,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -871,6 +886,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt "idp-id", "issuer", @@ -1012,6 +1028,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -1019,6 +1036,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -1159,6 +1177,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -1166,6 +1185,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -1306,6 +1326,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -1313,6 +1334,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -1454,6 +1476,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -1461,6 +1484,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -1601,6 +1625,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -1608,6 +1633,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -1752,6 +1778,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -1759,6 +1786,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -1920,6 +1948,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -1927,6 +1956,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -2069,6 +2099,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -2076,6 +2107,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -2246,6 +2278,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -2253,6 +2286,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -2423,6 +2457,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -2430,6 +2465,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -2573,6 +2609,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -2580,6 +2617,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -2688,6 +2726,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -2695,6 +2734,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -2803,6 +2843,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -2810,6 +2851,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -2918,6 +2960,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { "user", database.TextArray[string]{"profile"}, "id-attribute", + true, // oidc nil, nil, @@ -2925,6 +2968,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt nil, nil, @@ -3033,6 +3077,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc "idp-id-oidc", "issuer", @@ -3040,6 +3085,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, database.TextArray[string]{"profile"}, true, + true, // jwt nil, nil, @@ -3148,6 +3194,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // oidc nil, nil, @@ -3155,6 +3202,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { nil, nil, nil, + nil, // jwt "idp-id-jwt", "issuer", @@ -3363,6 +3411,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { UserEndpoint: "user", Scopes: []string{"profile"}, IDAttribute: "id-attribute", + UsePKCE: true, }, }, { @@ -3387,6 +3436,7 @@ func Test_IDPTemplateTemplatesPrepares(t *testing.T) { ClientSecret: nil, Scopes: []string{"profile"}, IsIDTokenMapping: true, + UsePKCE: true, }, }, { diff --git a/internal/query/projection/idp_template.go b/internal/query/projection/idp_template.go index a5edcba169..47033e6bbb 100644 --- a/internal/query/projection/idp_template.go +++ b/internal/query/projection/idp_template.go @@ -70,6 +70,7 @@ const ( OAuthUserEndpointCol = "user_endpoint" OAuthScopesCol = "scopes" OAuthIDAttributeCol = "id_attribute" + OAuthUsePKCECol = "use_pkce" OIDCIDCol = "idp_id" OIDCInstanceIDCol = "instance_id" @@ -78,6 +79,7 @@ const ( OIDCClientSecretCol = "client_secret" OIDCScopesCol = "scopes" OIDCIDTokenMappingCol = "id_token_mapping" + OIDCUsePKCECol = "use_pkce" JWTIDCol = "idp_id" JWTInstanceIDCol = "instance_id" @@ -217,6 +219,7 @@ func (*idpTemplateProjection) Init() *old_handler.Check { handler.NewColumn(OAuthUserEndpointCol, handler.ColumnTypeText), handler.NewColumn(OAuthScopesCol, handler.ColumnTypeTextArray, handler.Nullable()), handler.NewColumn(OAuthIDAttributeCol, handler.ColumnTypeText), + handler.NewColumn(OAuthUsePKCECol, handler.ColumnTypeBool, handler.Default(false)), }, handler.NewPrimaryKey(OAuthInstanceIDCol, OAuthIDCol), IDPTemplateOAuthSuffix, @@ -230,6 +233,7 @@ func (*idpTemplateProjection) Init() *old_handler.Check { handler.NewColumn(OIDCClientSecretCol, handler.ColumnTypeJSONB), handler.NewColumn(OIDCScopesCol, handler.ColumnTypeTextArray, handler.Nullable()), handler.NewColumn(OIDCIDTokenMappingCol, handler.ColumnTypeBool, handler.Default(false)), + handler.NewColumn(OIDCUsePKCECol, handler.ColumnTypeBool, handler.Default(false)), }, handler.NewPrimaryKey(OIDCInstanceIDCol, OIDCIDCol), IDPTemplateOIDCSuffix, @@ -722,6 +726,7 @@ func (p *idpTemplateProjection) reduceOAuthIDPAdded(event eventstore.Event) (*ha handler.NewCol(OAuthUserEndpointCol, idpEvent.UserEndpoint), handler.NewCol(OAuthScopesCol, database.TextArray[string](idpEvent.Scopes)), handler.NewCol(OAuthIDAttributeCol, idpEvent.IDAttribute), + handler.NewCol(OAuthUsePKCECol, idpEvent.UsePKCE), }, handler.WithTableSuffix(IDPTemplateOAuthSuffix), ), @@ -813,6 +818,7 @@ func (p *idpTemplateProjection) reduceOIDCIDPAdded(event eventstore.Event) (*han handler.NewCol(OIDCClientSecretCol, idpEvent.ClientSecret), handler.NewCol(OIDCScopesCol, database.TextArray[string](idpEvent.Scopes)), handler.NewCol(OIDCIDTokenMappingCol, idpEvent.IsIDTokenMapping), + handler.NewCol(OIDCUsePKCECol, idpEvent.UsePKCE), }, handler.WithTableSuffix(IDPTemplateOIDCSuffix), ), @@ -1154,6 +1160,7 @@ func (p *idpTemplateProjection) reduceOldOIDCConfigAdded(event eventstore.Event) handler.NewCol(OIDCClientSecretCol, idpEvent.ClientSecret), handler.NewCol(OIDCScopesCol, database.TextArray[string](idpEvent.Scopes)), handler.NewCol(OIDCIDTokenMappingCol, true), + handler.NewCol(OIDCUsePKCECol, false), }, handler.WithTableSuffix(IDPTemplateOIDCSuffix), ), @@ -2253,6 +2260,9 @@ func reduceOAuthIDPChangedColumns(idpEvent idp.OAuthIDPChangedEvent) []handler.C if idpEvent.IDAttribute != nil { oauthCols = append(oauthCols, handler.NewCol(OAuthIDAttributeCol, *idpEvent.IDAttribute)) } + if idpEvent.UsePKCE != nil { + oauthCols = append(oauthCols, handler.NewCol(OAuthUsePKCECol, *idpEvent.UsePKCE)) + } return oauthCols } @@ -2273,6 +2283,9 @@ func reduceOIDCIDPChangedColumns(idpEvent idp.OIDCIDPChangedEvent) []handler.Col if idpEvent.IsIDTokenMapping != nil { oidcCols = append(oidcCols, handler.NewCol(OIDCIDTokenMappingCol, *idpEvent.IsIDTokenMapping)) } + if idpEvent.UsePKCE != nil { + oidcCols = append(oidcCols, handler.NewCol(OIDCUsePKCECol, *idpEvent.UsePKCE)) + } return oidcCols } diff --git a/internal/query/projection/idp_template_test.go b/internal/query/projection/idp_template_test.go index e8efa82f87..74adfe22c0 100644 --- a/internal/query/projection/idp_template_test.go +++ b/internal/query/projection/idp_template_test.go @@ -192,6 +192,7 @@ func TestIDPTemplateProjection_reducesOAuth(t *testing.T) { "userEndpoint": "user", "scopes": ["profile"], "idAttribute": "id-attribute", + "usePKCE": false, "isCreationAllowed": true, "isLinkingAllowed": true, "isAutoCreation": true, @@ -227,7 +228,7 @@ func TestIDPTemplateProjection_reducesOAuth(t *testing.T) { }, }, { - expectedStmt: "INSERT INTO projections.idp_templates6_oauth2 (idp_id, instance_id, client_id, client_secret, authorization_endpoint, token_endpoint, user_endpoint, scopes, id_attribute) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)", + expectedStmt: "INSERT INTO projections.idp_templates6_oauth2 (idp_id, instance_id, client_id, client_secret, authorization_endpoint, token_endpoint, user_endpoint, scopes, id_attribute, use_pkce) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)", expectedArgs: []interface{}{ "idp-id", "instance-id", @@ -238,6 +239,7 @@ func TestIDPTemplateProjection_reducesOAuth(t *testing.T) { "user", database.TextArray[string]{"profile"}, "id-attribute", + false, }, }, }, @@ -265,6 +267,7 @@ func TestIDPTemplateProjection_reducesOAuth(t *testing.T) { "userEndpoint": "user", "scopes": ["profile"], "idAttribute": "id-attribute", + "usePKCE": true, "isCreationAllowed": true, "isLinkingAllowed": true, "isAutoCreation": true, @@ -300,7 +303,7 @@ func TestIDPTemplateProjection_reducesOAuth(t *testing.T) { }, }, { - expectedStmt: "INSERT INTO projections.idp_templates6_oauth2 (idp_id, instance_id, client_id, client_secret, authorization_endpoint, token_endpoint, user_endpoint, scopes, id_attribute) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)", + expectedStmt: "INSERT INTO projections.idp_templates6_oauth2 (idp_id, instance_id, client_id, client_secret, authorization_endpoint, token_endpoint, user_endpoint, scopes, id_attribute, use_pkce) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)", expectedArgs: []interface{}{ "idp-id", "instance-id", @@ -311,6 +314,7 @@ func TestIDPTemplateProjection_reducesOAuth(t *testing.T) { "user", database.TextArray[string]{"profile"}, "id-attribute", + true, }, }, }, @@ -380,6 +384,7 @@ func TestIDPTemplateProjection_reducesOAuth(t *testing.T) { "userEndpoint": "user", "scopes": ["profile"], "idAttribute": "id-attribute", + "usePKCE": true, "isCreationAllowed": true, "isLinkingAllowed": true, "isAutoCreation": true, @@ -410,7 +415,7 @@ func TestIDPTemplateProjection_reducesOAuth(t *testing.T) { }, }, { - expectedStmt: "UPDATE projections.idp_templates6_oauth2 SET (client_id, client_secret, authorization_endpoint, token_endpoint, user_endpoint, scopes, id_attribute) = ($1, $2, $3, $4, $5, $6, $7) WHERE (idp_id = $8) AND (instance_id = $9)", + expectedStmt: "UPDATE projections.idp_templates6_oauth2 SET (client_id, client_secret, authorization_endpoint, token_endpoint, user_endpoint, scopes, id_attribute, use_pkce) = ($1, $2, $3, $4, $5, $6, $7, $8) WHERE (idp_id = $9) AND (instance_id = $10)", expectedArgs: []interface{}{ "client_id", anyArg{}, @@ -419,6 +424,7 @@ func TestIDPTemplateProjection_reducesOAuth(t *testing.T) { "user", database.TextArray[string]{"profile"}, "id-attribute", + true, "idp-id", "instance-id", }, @@ -3081,6 +3087,7 @@ func TestIDPTemplateProjection_reducesOIDC(t *testing.T) { }, "scopes": ["profile"], "idTokenMapping": true, + "usePKCE": true, "isCreationAllowed": true, "isLinkingAllowed": true, "isAutoCreation": true, @@ -3116,7 +3123,7 @@ func TestIDPTemplateProjection_reducesOIDC(t *testing.T) { }, }, { - expectedStmt: "INSERT INTO projections.idp_templates6_oidc (idp_id, instance_id, issuer, client_id, client_secret, scopes, id_token_mapping) VALUES ($1, $2, $3, $4, $5, $6, $7)", + expectedStmt: "INSERT INTO projections.idp_templates6_oidc (idp_id, instance_id, issuer, client_id, client_secret, scopes, id_token_mapping, use_pkce) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)", expectedArgs: []interface{}{ "idp-id", "instance-id", @@ -3125,6 +3132,7 @@ func TestIDPTemplateProjection_reducesOIDC(t *testing.T) { anyArg{}, database.TextArray[string]{"profile"}, true, + true, }, }, }, @@ -3149,6 +3157,7 @@ func TestIDPTemplateProjection_reducesOIDC(t *testing.T) { }, "scopes": ["profile"], "idTokenMapping": true, + "usePKCE": true, "isCreationAllowed": true, "isLinkingAllowed": true, "isAutoCreation": true, @@ -3184,7 +3193,7 @@ func TestIDPTemplateProjection_reducesOIDC(t *testing.T) { }, }, { - expectedStmt: "INSERT INTO projections.idp_templates6_oidc (idp_id, instance_id, issuer, client_id, client_secret, scopes, id_token_mapping) VALUES ($1, $2, $3, $4, $5, $6, $7)", + expectedStmt: "INSERT INTO projections.idp_templates6_oidc (idp_id, instance_id, issuer, client_id, client_secret, scopes, id_token_mapping, use_pkce) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)", expectedArgs: []interface{}{ "idp-id", "instance-id", @@ -3193,6 +3202,7 @@ func TestIDPTemplateProjection_reducesOIDC(t *testing.T) { anyArg{}, database.TextArray[string]{"profile"}, true, + true, }, }, }, @@ -3260,6 +3270,7 @@ func TestIDPTemplateProjection_reducesOIDC(t *testing.T) { }, "scopes": ["profile"], "idTokenMapping": true, + "usePKCE": true, "isCreationAllowed": true, "isLinkingAllowed": true, "isAutoCreation": true, @@ -3290,13 +3301,14 @@ func TestIDPTemplateProjection_reducesOIDC(t *testing.T) { }, }, { - expectedStmt: "UPDATE projections.idp_templates6_oidc SET (client_id, client_secret, issuer, scopes, id_token_mapping) = ($1, $2, $3, $4, $5) WHERE (idp_id = $6) AND (instance_id = $7)", + expectedStmt: "UPDATE projections.idp_templates6_oidc SET (client_id, client_secret, issuer, scopes, id_token_mapping, use_pkce) = ($1, $2, $3, $4, $5, $6) WHERE (idp_id = $7) AND (instance_id = $8)", expectedArgs: []interface{}{ "client_id", anyArg{}, "issuer", database.TextArray[string]{"profile"}, true, + true, "idp-id", "instance-id", }, @@ -3810,7 +3822,7 @@ func TestIDPTemplateProjection_reducesOldConfig(t *testing.T) { }, }, { - expectedStmt: "INSERT INTO projections.idp_templates6_oidc (idp_id, instance_id, issuer, client_id, client_secret, scopes, id_token_mapping) VALUES ($1, $2, $3, $4, $5, $6, $7)", + expectedStmt: "INSERT INTO projections.idp_templates6_oidc (idp_id, instance_id, issuer, client_id, client_secret, scopes, id_token_mapping, use_pkce) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)", expectedArgs: []interface{}{ "idp-config-id", "instance-id", @@ -3819,6 +3831,7 @@ func TestIDPTemplateProjection_reducesOldConfig(t *testing.T) { anyArg{}, database.TextArray[string]{"profile"}, true, + false, }, }, }, @@ -3864,7 +3877,7 @@ func TestIDPTemplateProjection_reducesOldConfig(t *testing.T) { }, }, { - expectedStmt: "INSERT INTO projections.idp_templates6_oidc (idp_id, instance_id, issuer, client_id, client_secret, scopes, id_token_mapping) VALUES ($1, $2, $3, $4, $5, $6, $7)", + expectedStmt: "INSERT INTO projections.idp_templates6_oidc (idp_id, instance_id, issuer, client_id, client_secret, scopes, id_token_mapping, use_pkce) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)", expectedArgs: []interface{}{ "idp-config-id", "instance-id", @@ -3873,6 +3886,7 @@ func TestIDPTemplateProjection_reducesOldConfig(t *testing.T) { anyArg{}, database.TextArray[string]{"profile"}, true, + false, }, }, }, diff --git a/internal/repository/idp/oauth.go b/internal/repository/idp/oauth.go index 9b9b776082..e168459eea 100644 --- a/internal/repository/idp/oauth.go +++ b/internal/repository/idp/oauth.go @@ -18,6 +18,7 @@ type OAuthIDPAddedEvent struct { UserEndpoint string `json:"userEndpoint,omitempty"` Scopes []string `json:"scopes,omitempty"` IDAttribute string `json:"idAttribute,omitempty"` + UsePKCE bool `json:"usePKCE,omitempty"` Options } @@ -32,6 +33,7 @@ func NewOAuthIDPAddedEvent( userEndpoint, idAttribute string, scopes []string, + usePKCE bool, options Options, ) *OAuthIDPAddedEvent { return &OAuthIDPAddedEvent{ @@ -45,6 +47,7 @@ func NewOAuthIDPAddedEvent( UserEndpoint: userEndpoint, Scopes: scopes, IDAttribute: idAttribute, + UsePKCE: usePKCE, Options: options, } } @@ -82,6 +85,7 @@ type OAuthIDPChangedEvent struct { UserEndpoint *string `json:"userEndpoint,omitempty"` Scopes []string `json:"scopes,omitempty"` IDAttribute *string `json:"idAttribute,omitempty"` + UsePKCE *bool `json:"usePKCE,omitempty"` OptionChanges } @@ -158,6 +162,12 @@ func ChangeOAuthIDAttribute(idAttribute string) func(*OAuthIDPChangedEvent) { } } +func ChangeOAuthUsePKCE(usePKCE bool) func(*OAuthIDPChangedEvent) { + return func(e *OAuthIDPChangedEvent) { + e.UsePKCE = &usePKCE + } +} + func (e *OAuthIDPChangedEvent) Payload() interface{} { return e } diff --git a/internal/repository/idp/oidc.go b/internal/repository/idp/oidc.go index 0970129ceb..8c51baa6cf 100644 --- a/internal/repository/idp/oidc.go +++ b/internal/repository/idp/oidc.go @@ -16,6 +16,7 @@ type OIDCIDPAddedEvent struct { ClientSecret *crypto.CryptoValue `json:"clientSecret"` Scopes []string `json:"scopes,omitempty"` IsIDTokenMapping bool `json:"idTokenMapping,omitempty"` + UsePKCE bool `json:"usePKCE,omitempty"` Options } @@ -27,7 +28,7 @@ func NewOIDCIDPAddedEvent( clientID string, clientSecret *crypto.CryptoValue, scopes []string, - isIDTokenMapping bool, + isIDTokenMapping, usePKCE bool, options Options, ) *OIDCIDPAddedEvent { return &OIDCIDPAddedEvent{ @@ -39,6 +40,7 @@ func NewOIDCIDPAddedEvent( ClientSecret: clientSecret, Scopes: scopes, IsIDTokenMapping: isIDTokenMapping, + UsePKCE: usePKCE, Options: options, } } @@ -74,6 +76,7 @@ type OIDCIDPChangedEvent struct { ClientSecret *crypto.CryptoValue `json:"clientSecret,omitempty"` Scopes []string `json:"scopes,omitempty"` IsIDTokenMapping *bool `json:"idTokenMapping,omitempty"` + UsePKCE *bool `json:"usePKCE,omitempty"` OptionChanges } @@ -139,6 +142,12 @@ func ChangeOIDCIsIDTokenMapping(idTokenMapping bool) func(*OIDCIDPChangedEvent) } } +func ChangeOIDCUsePKCE(usePKCE bool) func(*OIDCIDPChangedEvent) { + return func(e *OIDCIDPChangedEvent) { + e.UsePKCE = &usePKCE + } +} + func (e *OIDCIDPChangedEvent) Payload() interface{} { return e } diff --git a/internal/repository/idpintent/intent.go b/internal/repository/idpintent/intent.go index 9ac1a875cc..27e6391f95 100644 --- a/internal/repository/idpintent/intent.go +++ b/internal/repository/idpintent/intent.go @@ -21,9 +21,10 @@ const ( type StartedEvent struct { eventstore.BaseEvent `json:"-"` - SuccessURL *url.URL `json:"successURL"` - FailureURL *url.URL `json:"failureURL"` - IDPID string `json:"idpId"` + SuccessURL *url.URL `json:"successURL"` + FailureURL *url.URL `json:"failureURL"` + IDPID string `json:"idpId"` + IDPArguments map[string]any `json:"idpArguments,omitempty"` } func NewStartedEvent( @@ -32,6 +33,7 @@ func NewStartedEvent( successURL, failureURL *url.URL, idpID string, + idpArguments map[string]any, ) *StartedEvent { return &StartedEvent{ BaseEvent: *eventstore.NewBaseEventForPush( @@ -39,9 +41,10 @@ func NewStartedEvent( aggregate, StartedEventType, ), - SuccessURL: successURL, - FailureURL: failureURL, - IDPID: idpID, + SuccessURL: successURL, + FailureURL: failureURL, + IDPID: idpID, + IDPArguments: idpArguments, } } diff --git a/internal/repository/instance/idp.go b/internal/repository/instance/idp.go index 130a8d3d61..6ab60c0dd5 100644 --- a/internal/repository/instance/idp.go +++ b/internal/repository/instance/idp.go @@ -56,6 +56,7 @@ func NewOAuthIDPAddedEvent( userEndpoint, idAttribute string, scopes []string, + usePKCE bool, options idp.Options, ) *OAuthIDPAddedEvent { @@ -75,6 +76,7 @@ func NewOAuthIDPAddedEvent( userEndpoint, idAttribute, scopes, + usePKCE, options, ), } @@ -137,7 +139,7 @@ func NewOIDCIDPAddedEvent( clientID string, clientSecret *crypto.CryptoValue, scopes []string, - isIDTokenMapping bool, + isIDTokenMapping, usePKCE bool, options idp.Options, ) *OIDCIDPAddedEvent { @@ -155,6 +157,7 @@ func NewOIDCIDPAddedEvent( clientSecret, scopes, isIDTokenMapping, + usePKCE, options, ), } diff --git a/internal/repository/org/idp.go b/internal/repository/org/idp.go index 9510814e20..0070f71a95 100644 --- a/internal/repository/org/idp.go +++ b/internal/repository/org/idp.go @@ -56,6 +56,7 @@ func NewOAuthIDPAddedEvent( userEndpoint, idAttribute string, scopes []string, + usePKCE bool, options idp.Options, ) *OAuthIDPAddedEvent { @@ -75,6 +76,7 @@ func NewOAuthIDPAddedEvent( userEndpoint, idAttribute, scopes, + usePKCE, options, ), } @@ -137,7 +139,7 @@ func NewOIDCIDPAddedEvent( clientID string, clientSecret *crypto.CryptoValue, scopes []string, - isIDTokenMapping bool, + isIDTokenMapping, usePKCE bool, options idp.Options, ) *OIDCIDPAddedEvent { @@ -155,6 +157,7 @@ func NewOIDCIDPAddedEvent( clientSecret, scopes, isIDTokenMapping, + usePKCE, options, ), } diff --git a/proto/zitadel/admin.proto b/proto/zitadel/admin.proto index 763633a810..4050e9cee0 100644 --- a/proto/zitadel/admin.proto +++ b/proto/zitadel/admin.proto @@ -6124,6 +6124,8 @@ message AddGenericOAuthProviderRequest { } ]; zitadel.idp.v1.Options provider_options = 9; + // Enable the use of Proof Key for Code Exchange (PKCE) for the OAuth2 flow. + bool use_pkce = 10; } message AddGenericOAuthProviderResponse { @@ -6191,6 +6193,8 @@ message UpdateGenericOAuthProviderRequest { } ]; zitadel.idp.v1.Options provider_options = 10; + // Enable the use of Proof Key for Code Exchange (PKCE) for the OAuth2 flow. + bool use_pkce = 11; } message UpdateGenericOAuthProviderResponse { @@ -6234,6 +6238,8 @@ message AddGenericOIDCProviderRequest { ]; zitadel.idp.v1.Options provider_options = 6; bool is_id_token_mapping = 7; + // Enable the use of Proof Key for Code Exchange (PKCE) for the OIDC flow. + bool use_pkce = 8; } message AddGenericOIDCProviderResponse { @@ -6285,6 +6291,8 @@ message UpdateGenericOIDCProviderRequest { ]; zitadel.idp.v1.Options provider_options = 7; bool is_id_token_mapping = 8; + // Enable the use of Proof Key for Code Exchange (PKCE) for the OIDC flow. + bool use_pkce = 9; } message UpdateGenericOIDCProviderResponse { diff --git a/proto/zitadel/idp.proto b/proto/zitadel/idp.proto index eecb9eae98..82e32aa873 100644 --- a/proto/zitadel/idp.proto +++ b/proto/zitadel/idp.proto @@ -338,6 +338,8 @@ message OAuthConfig { description: "defines how the attribute is called where ZITADEL can get the id of the user"; } ]; + // Defines if the Proof Key for Code Exchange (PKCE) is used for the authorization code flow. + bool use_pkce = 7; } message GenericOIDCConfig { @@ -365,6 +367,12 @@ message GenericOIDCConfig { description: "if true, provider information get mapped from the id token, not from the userinfo endpoint"; } ]; + // Defines if the Proof Key for Code Exchange (PKCE) is used for the authorization code flow. + bool use_pkce = 5 [ + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + example: "true"; + } + ]; } message GitHubConfig { diff --git a/proto/zitadel/management.proto b/proto/zitadel/management.proto index 818600efee..e69e331f87 100644 --- a/proto/zitadel/management.proto +++ b/proto/zitadel/management.proto @@ -12545,6 +12545,8 @@ message AddGenericOAuthProviderRequest { } ]; zitadel.idp.v1.Options provider_options = 9; + // Enable the use of Proof Key for Code Exchange (PKCE) for the OAuth2 flow. + bool use_pkce = 10; } message AddGenericOAuthProviderResponse { @@ -12612,6 +12614,8 @@ message UpdateGenericOAuthProviderRequest { } ]; zitadel.idp.v1.Options provider_options = 10; + // Enable the use of Proof Key for Code Exchange (PKCE) for the OAuth2 flow. + bool use_pkce = 11; } message UpdateGenericOAuthProviderResponse { @@ -12655,6 +12659,8 @@ message AddGenericOIDCProviderRequest { ]; zitadel.idp.v1.Options provider_options = 6; bool is_id_token_mapping = 7; + // Enable the use of Proof Key for Code Exchange (PKCE) for the OIDC flow. + bool use_pkce = 8; } message AddGenericOIDCProviderResponse { @@ -12706,6 +12712,8 @@ message UpdateGenericOIDCProviderRequest { ]; zitadel.idp.v1.Options provider_options = 7; bool is_id_token_mapping = 8; + // Enable the use of Proof Key for Code Exchange (PKCE) for the OIDC flow. + bool use_pkce = 9; } message UpdateGenericOIDCProviderResponse {