Skip to content

Commit

Permalink
feat: forward prompt upstream parameter during OIDC flow (#3276)
Browse files Browse the repository at this point in the history
Fixes #2709
  • Loading branch information
David-Wobrock authored May 25, 2023
1 parent c842a69 commit d290cb0
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 5 deletions.
6 changes: 5 additions & 1 deletion selfservice/strategy/oidc/.schema/link.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,12 @@
"description": "The hd (hosted domain) parameter streamlines the login process for G Suite hosted accounts. By including the domain of the G Suite user (for example, mycollege.edu), you can indicate that the account selection UI should be optimized for accounts at that domain.",
"type": "string"
},
"prompt": {
"description": "The prompt specifies whether the Authorization Server prompts the End-User for reauthentication and consent (for example, select_account).",
"type": "string"
},
"additionalProperties": false
}
}
}
}
}
6 changes: 5 additions & 1 deletion selfservice/strategy/oidc/.schema/settings.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,12 @@
"description": "The hd (hosted domain) parameter streamlines the login process for G Suite hosted accounts. By including the domain of the G Suite user (for example, mycollege.edu), you can indicate that the account selection UI should be optimized for accounts at that domain.",
"type": "string"
},
"prompt": {
"description": "The prompt specifies whether the Authorization Server prompts the End-User for reauthentication and consent (for example, select_account).",
"type": "string"
},
"additionalProperties": false
}
}
}
}
}
2 changes: 2 additions & 0 deletions selfservice/strategy/oidc/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ func (c *Claims) Validate() error {
// Allowed parameters are:
// - `login_hint` (string): The `login_hint` parameter suppresses the account chooser and either pre-fills the email box on the sign-in form, or selects the proper session.
// - `hd` (string): The `hd` parameter limits the login/registration process to a Google Organization, e.g. `mycollege.edu`.
// - `prompt` (string): The `prompt` specifies whether the Authorization Server prompts the End-User for reauthentication and consent, e.g. `select_account`.
func UpstreamParameters(provider Provider, upstreamParameters map[string]string) []oauth2.AuthCodeOption {
// validation of upstream parameters are already handled in the `oidc/.schema/link.schema.json` and `oidc/.schema/settings.schema.json` file.
// `upstreamParameters` will always only contain allowed parameters based on the configuration.
Expand All @@ -83,6 +84,7 @@ func UpstreamParameters(provider Provider, upstreamParameters map[string]string)
allowedParameters := map[string]struct{}{
"login_hint": {},
"hd": {},
"prompt": {},
}

var params []oauth2.AuthCodeOption
Expand Down
3 changes: 2 additions & 1 deletion selfservice/strategy/oidc/strategy_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ type UpdateLoginFlowWithOidcMethod struct {
// Supported parameters are:
// - `login_hint` (string): The `login_hint` parameter suppresses the account chooser and either pre-fills the email box on the sign-in form, or selects the proper session.
// - `hd` (string): The `hd` parameter limits the login/registration process to a Google Organization, e.g. `mycollege.edu`.
// - `prompt` (string): The `prompt` specifies whether the Authorization Server prompts the End-User for reauthentication and consent, e.g. `select_account`.
//
// required: false
UpstreamParameters json.RawMessage `json:"upstream_parameters"`
Expand Down Expand Up @@ -211,7 +212,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow,
return nil, err
}

codeURL := c.AuthCodeURL(state.String(), append(provider.AuthCodeURLOptions(req), UpstreamParameters(provider, up)...)...)
codeURL := c.AuthCodeURL(state.String(), append(UpstreamParameters(provider, up), provider.AuthCodeURLOptions(req)...)...)
if x.IsJSONRequest(r) {
s.d.Writer().WriteError(w, r, flow.NewBrowserLocationChangeRequiredError(codeURL))
} else {
Expand Down
3 changes: 2 additions & 1 deletion selfservice/strategy/oidc/strategy_registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ type UpdateRegistrationFlowWithOidcMethod struct {
// Supported parameters are:
// - `login_hint` (string): The `login_hint` parameter suppresses the account chooser and either pre-fills the email box on the sign-in form, or selects the proper session.
// - `hd` (string): The `hd` parameter limits the login/registration process to a Google Organization, e.g. `mycollege.edu`.
// - `prompt` (string): The `prompt` specifies whether the Authorization Server prompts the End-User for reauthentication and consent, e.g. `select_account`.
//
// required: false
UpstreamParameters json.RawMessage `json:"upstream_parameters"`
Expand Down Expand Up @@ -176,7 +177,7 @@ func (s *Strategy) Register(w http.ResponseWriter, r *http.Request, f *registrat
return err
}

codeURL := c.AuthCodeURL(state.String(), append(provider.AuthCodeURLOptions(req), UpstreamParameters(provider, up)...)...)
codeURL := c.AuthCodeURL(state.String(), append(UpstreamParameters(provider, up), provider.AuthCodeURLOptions(req)...)...)
if x.IsJSONRequest(r) {
s.d.Writer().WriteError(w, r, flow.NewBrowserLocationChangeRequiredError(codeURL))
} else {
Expand Down
3 changes: 2 additions & 1 deletion selfservice/strategy/oidc/strategy_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ type updateSettingsFlowWithOidcMethod struct {
// Supported parameters are:
// - `login_hint` (string): The `login_hint` parameter suppresses the account chooser and either pre-fills the email box on the sign-in form, or selects the proper session.
// - `hd` (string): The `hd` parameter limits the login/registration process to a Google Organization, e.g. `mycollege.edu`.
// - `prompt` (string): The `prompt` specifies whether the Authorization Server prompts the End-User for reauthentication and consent, e.g. `select_account`.
//
// required: false
UpstreamParameters json.RawMessage `json:"upstream_parameters"`
Expand Down Expand Up @@ -372,7 +373,7 @@ func (s *Strategy) initLinkProvider(w http.ResponseWriter, r *http.Request, ctxU
return err
}

codeURL := c.AuthCodeURL(state, append(provider.AuthCodeURLOptions(req), UpstreamParameters(provider, up)...)...)
codeURL := c.AuthCodeURL(state, append(UpstreamParameters(provider, up), provider.AuthCodeURLOptions(req)...)...)
if x.IsJSONRequest(r) {
s.d.Writer().WriteError(w, r, flow.NewBrowserLocationChangeRequiredError(codeURL))
} else {
Expand Down
2 changes: 2 additions & 0 deletions selfservice/strategy/oidc/strategy_settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ func TestSettingsStrategy(t *testing.T) {
values.Set("link", provider)
values.Set("upstream_parameters.login_hint", "[email protected]")
values.Set("upstream_parameters.hd", "bar.com")
values.Set("upstream_parameters.prompt", "consent")

resp, err := c.PostForm(action(req), *values)
require.NoError(t, err)
Expand All @@ -495,6 +496,7 @@ func TestSettingsStrategy(t *testing.T) {

require.EqualValues(t, "[email protected]", loc.Query().Get("login_hint"))
require.EqualValues(t, "bar.com", loc.Query().Get("hd"))
require.EqualValues(t, "consent", loc.Query().Get("prompt"))
})

t.Run("case=invalid query parameters should be ignored", func(t *testing.T) {
Expand Down
4 changes: 4 additions & 0 deletions selfservice/strategy/oidc/strategy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,7 @@ func TestStrategy(t *testing.T) {
fv.Set("provider", "valid")
fv.Set("upstream_parameters.login_hint", "[email protected]")
fv.Set("upstream_parameters.hd", "ory.sh")
fv.Set("upstream_parameters.prompt", "select_account")

res, err := c.PostForm(action, fv)
require.NoError(t, err)
Expand All @@ -681,6 +682,7 @@ func TestStrategy(t *testing.T) {

require.Equal(t, "[email protected]", loc.Query().Get("login_hint"))
require.Equal(t, "ory.sh", loc.Query().Get("hd"))
require.Equal(t, "select_account", loc.Query().Get("prompt"))
})

t.Run("case=should pass when logging in", func(t *testing.T) {
Expand All @@ -693,6 +695,7 @@ func TestStrategy(t *testing.T) {
fv.Set("provider", "valid")
fv.Set("upstream_parameters.login_hint", "[email protected]")
fv.Set("upstream_parameters.hd", "ory.sh")
fv.Set("upstream_parameters.prompt", "select_account")

res, err := c.PostForm(action, fv)
require.NoError(t, err)
Expand All @@ -703,6 +706,7 @@ func TestStrategy(t *testing.T) {

require.Equal(t, "[email protected]", loc.Query().Get("login_hint"))
require.Equal(t, "ory.sh", loc.Query().Get("hd"))
require.Equal(t, "select_account", loc.Query().Get("prompt"))
})

t.Run("case=should ignore invalid parameters when logging in", func(t *testing.T) {
Expand Down

0 comments on commit d290cb0

Please sign in to comment.