From ca6bd9d6a2d07d744e9a8fe9108db3e0986308a6 Mon Sep 17 00:00:00 2001 From: Wim Van Laer Date: Fri, 13 Feb 2026 15:34:32 +0100 Subject: [PATCH] fix: added `login_hint` to the idp intent (#11552) # Which Problems Are Solved In login V2, when the user started the login flow of an external IDP, there was no login_hint passed to the external provider. This required the user to enter their username again. Whilst they already entered it in Zitadel. By adding the `login_hint` parameter, the username should already be filled in on the external idp. # How the Problems Are Solved Pass the `login_hint` parameter to the external idp. # Additional Changes Also added the `login_hint` to the event-store for audit trailing. # Additional Context - Closes #11392 --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> --- .../user/v2/integration_test/intent_test.go | 106 ++++++++++++++++++ internal/api/grpc/user/v2/intent.go | 10 +- internal/api/grpc/user/v2beta/user.go | 10 +- internal/command/idp_intent.go | 15 ++- internal/command/idp_intent_model.go | 2 + internal/command/idp_intent_test.go | 79 ++++++++++++- internal/command/session_test.go | 1 + internal/integration/sink/server.go | 2 +- internal/repository/idpintent/intent.go | 3 + proto/zitadel/user/v2/idp.proto | 8 ++ proto/zitadel/user/v2beta/idp.proto | 8 ++ 11 files changed, 225 insertions(+), 19 deletions(-) diff --git a/internal/api/grpc/user/v2/integration_test/intent_test.go b/internal/api/grpc/user/v2/integration_test/intent_test.go index f7fde1c65..bdb1844e4 100644 --- a/internal/api/grpc/user/v2/integration_test/intent_test.go +++ b/internal/api/grpc/user/v2/integration_test/intent_test.go @@ -1038,3 +1038,109 @@ func TestServer_RetrieveIdentityProviderIntent(t *testing.T) { }) } } + +func TestServer_StartIdentityProviderIntent_WithLoginHint(t *testing.T) { + oauthIdpResp := Instance.AddGenericOAuthProvider(IamCTX, integration.IDPName()) + + type args struct { + ctx context.Context + req *user.StartIdentityProviderIntentRequest + } + type want struct { + url string + parametersExisting []string + parametersEqual map[string]string + } + tests := []struct { + name string + args args + want want + wantErr bool + }{ + { + name: "OAuth with login_hint", + args: args{ + OrgCTX, + &user.StartIdentityProviderIntentRequest{ + IdpId: oauthIdpResp.Id, + Content: &user.StartIdentityProviderIntentRequest_Urls{ + Urls: &user.RedirectURLs{ + SuccessUrl: "https://example.com/success", + FailureUrl: "https://example.com/failure", + LoginHint: "user@example.com", + }, + }, + }, + }, + want: want{ + url: "https://example.com/oauth/v2/authorize", + parametersEqual: map[string]string{ + "client_id": "clientID", + "redirect_uri": "http://" + Instance.Domain + ":8082/idps/callback", + "response_type": "code", + "scope": "openid profile email", + "login_hint": "user@example.com", + }, + parametersExisting: []string{"state"}, + }, + wantErr: false, + }, + { + name: "Without login_hint", + args: args{ + OrgCTX, + &user.StartIdentityProviderIntentRequest{ + IdpId: oauthIdpResp.Id, + Content: &user.StartIdentityProviderIntentRequest_Urls{ + Urls: &user.RedirectURLs{ + SuccessUrl: "https://example.com/success", + FailureUrl: "https://example.com/failure", + // No LoginHint provided + }, + }, + }, + }, + want: want{ + url: "https://example.com/oauth/v2/authorize", + parametersEqual: map[string]string{ + "client_id": "clientID", + "prompt": "select_account", + "redirect_uri": "http://" + Instance.Domain + ":8082/idps/callback", + "response_type": "code", + "scope": "openid profile email", + }, + parametersExisting: []string{"state"}, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Client.StartIdentityProviderIntent(tt.args.ctx, tt.args.req) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + + authUrl, err := url.Parse(got.GetAuthUrl()) + require.NoError(t, err) + + assert.Equal(t, tt.want.url, authUrl.Scheme+"://"+authUrl.Host+authUrl.Path) + + for _, param := range tt.want.parametersExisting { + assert.True(t, authUrl.Query().Has(param), "Expected parameter %s to exist", param) + } + + for key, expectedValue := range tt.want.parametersEqual { + actualValue := authUrl.Query().Get(key) + assert.Equal(t, expectedValue, actualValue, "Parameter %s should have value %s but got %s", key, expectedValue, actualValue) + } + + if tt.args.req.GetContent().(*user.StartIdentityProviderIntentRequest_Urls).Urls.LoginHint == "" { + assert.False(t, authUrl.Query().Has("login_hint"), "login_hint should not be in URL when not provided or empty") + } + }) + } +} diff --git a/internal/api/grpc/user/v2/intent.go b/internal/api/grpc/user/v2/intent.go index 850776729..77cc3874b 100644 --- a/internal/api/grpc/user/v2/intent.go +++ b/internal/api/grpc/user/v2/intent.go @@ -36,7 +36,7 @@ import ( func (s *Server) StartIdentityProviderIntent(ctx context.Context, req *connect.Request[user.StartIdentityProviderIntentRequest]) (_ *connect.Response[user.StartIdentityProviderIntentResponse], err error) { switch t := req.Msg.GetContent().(type) { case *user.StartIdentityProviderIntentRequest_Urls: - return s.startIDPIntent(ctx, req.Msg.GetIdpId(), t.Urls) + return s.startIDPIntent(ctx, req.Msg.GetIdpId(), t.Urls, t.Urls.LoginHint) case *user.StartIdentityProviderIntentRequest_Ldap: return s.startLDAPIntent(ctx, req.Msg.GetIdpId(), t.Ldap) default: @@ -44,12 +44,12 @@ func (s *Server) StartIdentityProviderIntent(ctx context.Context, req *connect.R } } -func (s *Server) startIDPIntent(ctx context.Context, idpID string, urls *user.RedirectURLs) (*connect.Response[user.StartIdentityProviderIntentResponse], error) { - state, session, err := s.command.AuthFromProvider(ctx, idpID, s.idpCallback(ctx), s.samlRootURL(ctx, idpID)) +func (s *Server) startIDPIntent(ctx context.Context, idpID string, urls *user.RedirectURLs, loginHint string) (*connect.Response[user.StartIdentityProviderIntentResponse], error) { + state, session, err := s.command.AuthFromProvider(ctx, idpID, s.idpCallback(ctx), s.samlRootURL(ctx, idpID), loginHint) if err != nil { return nil, err } - _, details, err := s.command.CreateIntent(ctx, state, idpID, urls.GetSuccessUrl(), urls.GetFailureUrl(), authz.GetInstance(ctx).InstanceID(), session.PersistentParameters()) + _, details, err := s.command.CreateIntent(ctx, state, idpID, urls.GetSuccessUrl(), urls.GetFailureUrl(), authz.GetInstance(ctx).InstanceID(), loginHint, session.PersistentParameters()) if err != nil { return nil, err } @@ -78,7 +78,7 @@ func (s *Server) startIDPIntent(ctx context.Context, idpID string, urls *user.Re } func (s *Server) startLDAPIntent(ctx context.Context, idpID string, ldapCredentials *user.LDAPCredentials) (*connect.Response[user.StartIdentityProviderIntentResponse], error) { - intentWriteModel, details, err := s.command.CreateIntent(ctx, "", idpID, "", "", authz.GetInstance(ctx).InstanceID(), nil) + 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 44c6a2f4f..6dd848dd4 100644 --- a/internal/api/grpc/user/v2beta/user.go +++ b/internal/api/grpc/user/v2beta/user.go @@ -368,7 +368,7 @@ func userGrantsToIDs(userGrants []*query.UserGrant) []string { func (s *Server) StartIdentityProviderIntent(ctx context.Context, req *connect.Request[user.StartIdentityProviderIntentRequest]) (_ *connect.Response[user.StartIdentityProviderIntentResponse], err error) { switch t := req.Msg.GetContent().(type) { case *user.StartIdentityProviderIntentRequest_Urls: - return s.startIDPIntent(ctx, req.Msg.GetIdpId(), t.Urls) + return s.startIDPIntent(ctx, req.Msg.GetIdpId(), t.Urls, t.Urls.LoginHint) case *user.StartIdentityProviderIntentRequest_Ldap: return s.startLDAPIntent(ctx, req.Msg.GetIdpId(), t.Ldap) default: @@ -376,12 +376,12 @@ func (s *Server) StartIdentityProviderIntent(ctx context.Context, req *connect.R } } -func (s *Server) startIDPIntent(ctx context.Context, idpID string, urls *user.RedirectURLs) (*connect.Response[user.StartIdentityProviderIntentResponse], error) { - state, session, err := s.command.AuthFromProvider(ctx, idpID, s.idpCallback(ctx), s.samlRootURL(ctx, idpID)) +func (s *Server) startIDPIntent(ctx context.Context, idpID string, urls *user.RedirectURLs, loginHint string) (*connect.Response[user.StartIdentityProviderIntentResponse], error) { + state, session, err := s.command.AuthFromProvider(ctx, idpID, s.idpCallback(ctx), s.samlRootURL(ctx, idpID), loginHint) if err != nil { return nil, err } - _, details, err := s.command.CreateIntent(ctx, state, idpID, urls.GetSuccessUrl(), urls.GetFailureUrl(), authz.GetInstance(ctx).InstanceID(), session.PersistentParameters()) + _, details, err := s.command.CreateIntent(ctx, state, idpID, urls.GetSuccessUrl(), urls.GetFailureUrl(), authz.GetInstance(ctx).InstanceID(), loginHint, session.PersistentParameters()) if err != nil { return nil, err } @@ -410,7 +410,7 @@ func (s *Server) startIDPIntent(ctx context.Context, idpID string, urls *user.Re } func (s *Server) startLDAPIntent(ctx context.Context, idpID string, ldapCredentials *user.LDAPCredentials) (*connect.Response[user.StartIdentityProviderIntentResponse], error) { - intentWriteModel, details, err := s.command.CreateIntent(ctx, "", idpID, "", "", authz.GetInstance(ctx).InstanceID(), nil) + intentWriteModel, details, err := s.command.CreateIntent(ctx, "", idpID, "", "", authz.GetInstance(ctx).InstanceID(), "", nil) if err != nil { return nil, err } diff --git a/internal/command/idp_intent.go b/internal/command/idp_intent.go index 36948a6fd..97b460fc3 100644 --- a/internal/command/idp_intent.go +++ b/internal/command/idp_intent.go @@ -27,7 +27,7 @@ import ( "github.com/zitadel/zitadel/internal/zerrors" ) -func (c *Commands) prepareCreateIntent(writeModel *IDPIntentWriteModel, idpID, successURL, failureURL string, idpArguments map[string]any) preparation.Validation { +func (c *Commands) prepareCreateIntent(writeModel *IDPIntentWriteModel, idpID, successURL, failureURL string, loginHint 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") @@ -55,6 +55,7 @@ func (c *Commands) prepareCreateIntent(writeModel *IDPIntentWriteModel, idpID, s successURL, failureURL, idpID, + loginHint, idpArguments, ), }, nil @@ -62,7 +63,7 @@ func (c *Commands) prepareCreateIntent(writeModel *IDPIntentWriteModel, idpID, s } } -func (c *Commands) CreateIntent(ctx context.Context, intentID, idpID, successURL, failureURL, resourceOwner string, idpArguments map[string]any) (*IDPIntentWriteModel, *domain.ObjectDetails, error) { +func (c *Commands) CreateIntent(ctx context.Context, intentID, idpID, successURL, failureURL, resourceOwner, loginHint string, idpArguments map[string]any) (*IDPIntentWriteModel, *domain.ObjectDetails, error) { if intentID == "" { var err error intentID, err = c.idGenerator.Next() @@ -73,7 +74,7 @@ func (c *Commands) CreateIntent(ctx context.Context, intentID, idpID, successURL writeModel := NewIDPIntentWriteModel(intentID, resourceOwner, c.maxIdPIntentLifetime) //nolint: staticcheck - cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, c.prepareCreateIntent(writeModel, idpID, successURL, failureURL, idpArguments)) + cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, c.prepareCreateIntent(writeModel, idpID, successURL, failureURL, loginHint, idpArguments)) if err != nil { return nil, nil, err } @@ -135,7 +136,7 @@ func (c *Commands) GetActiveIntent(ctx context.Context, intentID string) (*IDPIn return intent, nil } -func (c *Commands) AuthFromProvider(ctx context.Context, idpID, idpCallback, samlRootURL string) (state string, session idp.Session, err error) { +func (c *Commands) AuthFromProvider(ctx context.Context, idpID, idpCallback, samlRootURL string, loginHint string) (state string, session idp.Session, err error) { state, err = c.idGenerator.Next() if err != nil { return "", nil, err @@ -144,7 +145,11 @@ func (c *Commands) AuthFromProvider(ctx context.Context, idpID, idpCallback, sam if err != nil { return "", nil, err } - session, err = provider.BeginAuth(ctx, state) + var params []idp.Parameter + if loginHint != "" { + params = append(params, idp.LoginHintParam(loginHint)) + } + session, err = provider.BeginAuth(ctx, state, params...) return state, session, err } diff --git a/internal/command/idp_intent_model.go b/internal/command/idp_intent_model.go index 07e082181..0155114f4 100644 --- a/internal/command/idp_intent_model.go +++ b/internal/command/idp_intent_model.go @@ -21,6 +21,7 @@ type IDPIntentWriteModel struct { IDPUserID string IDPUserName string UserID string + LoginHint string IDPAccessToken *crypto.CryptoValue IDPIDToken string @@ -97,6 +98,7 @@ func (wm *IDPIntentWriteModel) reduceStartedEvent(e *idpintent.StartedEvent) { wm.SuccessURL = e.SuccessURL wm.FailureURL = e.FailureURL wm.IDPID = e.IDPID + wm.LoginHint = e.LoginHint 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 000a48025..8114b0014 100644 --- a/internal/command/idp_intent_test.go +++ b/internal/command/idp_intent_test.go @@ -49,6 +49,7 @@ func TestCommands_CreateIntent(t *testing.T) { successURL string failureURL string instanceID string + loginHint string idpArguments map[string]any } type res struct { @@ -202,6 +203,7 @@ func TestCommands_CreateIntent(t *testing.T) { success, failure, "idp", + "", nil, ) }(), @@ -257,6 +259,7 @@ func TestCommands_CreateIntent(t *testing.T) { success, failure, "idp", + "", map[string]interface{}{ "verifier": "pkceOAuthVerifier", }, @@ -317,6 +320,7 @@ func TestCommands_CreateIntent(t *testing.T) { success, failure, "idp", + "", map[string]interface{}{ "verifier": "pkceOAuthVerifier", }, @@ -377,6 +381,7 @@ func TestCommands_CreateIntent(t *testing.T) { success, failure, "idp", + "", map[string]interface{}{ "verifier": "pkceOAuthVerifier", }, @@ -408,7 +413,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.intentID, tt.args.idpID, tt.args.successURL, tt.args.failureURL, tt.args.instanceID, tt.args.idpArguments) + 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.loginHint, tt.args.idpArguments) require.ErrorIs(t, err, tt.res.err) if intentWriteModel != nil { assert.Equal(t, tt.res.intentID, intentWriteModel.AggregateID) @@ -429,6 +434,7 @@ func TestCommands_AuthFromProvider(t *testing.T) { type args struct { ctx context.Context idpID string + loginHint string callbackURL string samlRootURL string } @@ -582,6 +588,68 @@ func TestCommands_AuthFromProvider(t *testing.T) { auth: &idp.RedirectAuth{RedirectURL: "auth?client_id=clientID&prompt=select_account&redirect_uri=url&response_type=code&state=id"}, }, }, + { + "oauth auth redirect with login_hint", + fields{ + secretCrypto: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + eventstore: expectEventstore( + expectFilter( + eventFromEventPusherWithInstanceID( + "instance", + 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{}, + )), + ), + expectFilter( + eventFromEventPusherWithInstanceID( + "instance", + 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{}, + )), + ), + ), + idGenerator: mock.ExpectID(t, "id"), + }, + args{ + ctx: authz.SetCtxData(context.Background(), authz.CtxData{OrgID: "ro"}), + idpID: "idp", + callbackURL: "url", + loginHint: "user@example.com", + }, + res{ + auth: &idp.RedirectAuth{RedirectURL: "auth?client_id=clientID&login_hint=user%40example.com&redirect_uri=url&response_type=code&state=id"}, + }, + }, { "migrated and push", fields{ @@ -681,7 +749,7 @@ func TestCommands_AuthFromProvider(t *testing.T) { idpConfigEncryption: tt.fields.secretCrypto, idGenerator: tt.fields.idGenerator, } - _, session, err := c.AuthFromProvider(tt.args.ctx, tt.args.idpID, tt.args.callbackURL, tt.args.samlRootURL) + _, session, err := c.AuthFromProvider(tt.args.ctx, tt.args.idpID, tt.args.callbackURL, tt.args.samlRootURL, tt.args.loginHint) require.ErrorIs(t, err, tt.res.err) var got idp.Auth @@ -705,6 +773,7 @@ func TestCommands_AuthFromProvider_SAML(t *testing.T) { idpID string callbackURL string samlRootURL string + loginHint string } type res struct { url string @@ -779,6 +848,7 @@ func TestCommands_AuthFromProvider_SAML(t *testing.T) { success, failure, "idp", + "hint@example.com", nil, ) }(), @@ -801,6 +871,7 @@ func TestCommands_AuthFromProvider_SAML(t *testing.T) { idpID: "idp", callbackURL: "url", samlRootURL: "samlurl", + loginHint: "hint@example.com", }, res{ url: "http://localhost:8000/sso", @@ -872,6 +943,7 @@ func TestCommands_AuthFromProvider_SAML(t *testing.T) { success, failure, "idp", + "user123", nil, ) }(), @@ -894,6 +966,7 @@ func TestCommands_AuthFromProvider_SAML(t *testing.T) { idpID: "idp", callbackURL: "url", samlRootURL: "samlurl", + loginHint: "user123", }, res{ url: "http://localhost:8000/sso", @@ -911,7 +984,7 @@ func TestCommands_AuthFromProvider_SAML(t *testing.T) { idpConfigEncryption: tt.fields.secretCrypto, idGenerator: tt.fields.idGenerator, } - _, session, err := c.AuthFromProvider(tt.args.ctx, tt.args.idpID, tt.args.callbackURL, tt.args.samlRootURL) + _, session, err := c.AuthFromProvider(tt.args.ctx, tt.args.idpID, tt.args.callbackURL, tt.args.samlRootURL, tt.args.loginHint) require.ErrorIs(t, err, tt.res.err) auth, err := session.GetAuth(tt.args.ctx) diff --git a/internal/command/session_test.go b/internal/command/session_test.go index 0d0e5b043..09c136e4e 100644 --- a/internal/command/session_test.go +++ b/internal/command/session_test.go @@ -993,6 +993,7 @@ func TestCommands_updateSession(t *testing.T) { nil, nil, "idpID", + "", nil, ), ), diff --git a/internal/integration/sink/server.go b/internal/integration/sink/server.go index 3ce50c1f4..e8f81d51d 100644 --- a/internal/integration/sink/server.go +++ b/internal/integration/sink/server.go @@ -413,7 +413,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, nil) + 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/repository/idpintent/intent.go b/internal/repository/idpintent/intent.go index e4ee28cae..91f907781 100644 --- a/internal/repository/idpintent/intent.go +++ b/internal/repository/idpintent/intent.go @@ -26,6 +26,7 @@ type StartedEvent struct { SuccessURL *url.URL `json:"successURL"` FailureURL *url.URL `json:"failureURL"` IDPID string `json:"idpId"` + LoginHint string `json:"loginHint"` IDPArguments map[string]any `json:"idpArguments,omitempty"` } @@ -35,6 +36,7 @@ func NewStartedEvent( successURL, failureURL *url.URL, idpID string, + loginHint string, idpArguments map[string]any, ) *StartedEvent { return &StartedEvent{ @@ -46,6 +48,7 @@ func NewStartedEvent( SuccessURL: successURL, FailureURL: failureURL, IDPID: idpID, + LoginHint: loginHint, IDPArguments: idpArguments, } } diff --git a/proto/zitadel/user/v2/idp.proto b/proto/zitadel/user/v2/idp.proto index b2499ed78..eeb51106a 100644 --- a/proto/zitadel/user/v2/idp.proto +++ b/proto/zitadel/user/v2/idp.proto @@ -49,6 +49,14 @@ message RedirectURLs { example: "\"https://custom.com/login/idp/fail\""; } ]; + string login_hint = 3 [ + (validate.rules).string = {max_len: 200}, + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + description: "login_hint hints to the login page which user wants to log in." + max_length: 200; + example: "\"someone@example.com\""; + } + ]; } message IDPIntent { diff --git a/proto/zitadel/user/v2beta/idp.proto b/proto/zitadel/user/v2beta/idp.proto index 32e0c5f32..96c34863f 100644 --- a/proto/zitadel/user/v2beta/idp.proto +++ b/proto/zitadel/user/v2beta/idp.proto @@ -49,6 +49,14 @@ message RedirectURLs { example: "\"https://custom.com/login/idp/fail\""; } ]; + string login_hint = 3 [ + (validate.rules).string = {max_len: 200}, + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + description: "login_hint hints to the login page which user wants to log in." + max_length: 200; + example: "\"someone@example.com\""; + } + ]; } message IDPIntent {