feat: add upstream parameters to oidc provider

[Benehiko]

This PR introduces the upstream OIDC query parameters login_hint and hd. More can be added at a later stage in the link.schema.json and settings.schema.json file.

kratos/selfservice/strategy/oidc/.schema/link.schema.json

Lines 20 to 31 in 8fe11cd

	"upstream_parameters": {
	"type": "object",
	"$comment": "Only the defined parameters are allowed. This is to prevent users from sending arbitrary parameters or craft URLs that cause unexpected behavior.",
	"properties": {
	"login_hint": {
	"description": "The login hint could be an email address or identifier in the upstream provider to pre-fill the upstream provider login form",
	"type": "string"
	},
	"hd": {
	"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"
	},

https://github.com/ory/kratos/blob/4362423c3cddd98e46745493288fec69f9c66766/selfservice/strategy/oidc/.schema/settings.schema.json

To send additional upstream parameters the form can post this on a login, registration or settings link submit.
For example the form below does an OIDC flow to Google. We can now add additional parameters such as login_hint and hd to the upstream request to Google login with a pre-filled email [email protected]:

<form action="https://kratos/self-service/login?flow=">
  <input type="submit" name="provider" value="google" />
  <input type="hidden" name="upstream_parameters.login_hint" value="[email protected]" />
  <input type="hidden" name="upstream_parameters.hd" value="example.com" />
</form>

Related issue(s)

#3127
#2069

Checklist

• I have read the contributing guidelines.
• I have referenced an issue containing the design document if my change
  introduces a new feature.
• I am following the
  contributing code guidelines.
• I have read the security policy.
• I confirm that this pull request does not address a security
  vulnerability. If this pull request addresses a security vulnerability, I
  confirm that I got the approval (please contact
  [email protected]) from the maintainers to push
  the changes.
• I have added tests that prove my fix is effective or that my feature
  works.
• I have added or changed the documentation.

Further Comments

[codecov]

Codecov Report

Merging #3138 (af8ccac) into master (dba3803) will decrease coverage by 0.03%.
The diff coverage is 72.72%.

❗ Current head af8ccac differs from pull request most recent head 4362423. Consider uploading reports for the commit 4362423 to get more accurate results

@@            Coverage Diff             @@
##           master    #3138      +/-   ##
==========================================
- Coverage   77.58%   77.55%   -0.03%     
==========================================
  Files         316      316              
  Lines       19906    19924      +18     
==========================================
+ Hits        15444    15452       +8     
- Misses       3273     3279       +6     
- Partials     1189     1193       +4     

Impacted Files	Coverage Δ
selfservice/strategy/oidc/strategy_login.go	66.25% <50.00%> (-1.29%)	⬇️
selfservice/strategy/oidc/strategy_registration.go	64.81% <50.00%> (-0.60%)	⬇️
selfservice/strategy/oidc/strategy_settings.go	63.77% <50.00%> (-0.35%)	⬇️
selfservice/flow/login/handler.go	78.77% <100.00%> (ø)
selfservice/strategy/oidc/provider.go	100.00% <100.00%> (ø)
session/test/persistence.go	99.05% <0.00%> (-0.95%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[Benehiko]

For the E2E tests to pass here we need to merge ory/kratos-selfservice-ui-react-nextjs#54 first.

Locally e2e tests pass.

[jonas-jonas]

LGTM.

[CaptainStandby]

Nice work. Only some minor things.

[aeneasr]

Nicely done, I have a some comments :)

[aeneasr]

I'd prefer this to be more explicit, because the input source probably is validated against the form payload, but it might not be in a certain case (e.g. the flow is resumed or something). By putting the validation in the central place where the code is executed, the input source no longer matters because the validation is "deeply embedded" into the flow, and not on the "surface".

The tests cover this nicely already, but for example they don't cover the account settings flow. So here we would have uncertainty whether this validation actually is implemented / working.

Generally speaking, security validation should always be very very explicit and treated like "do you actually have this permission and do I have 100% certainty?".

[Benehiko]

I believe this should now be addressed. I have added the upstream parameters to Settings flow as well. I am also double checking the parameters inside the UpstreamParameters function.

[aeneasr]

@Benehiko can you please add this to the google social sign in documentation?