OAuth2 Authorization Code Grant Flow #247

johakoch · 2021-05-26T07:20:56Z

No description provided.

johakoch · 2021-06-14T08:38:17Z

Im Moment ist es so implementiert, dass erwartet wird, dass Code-Verifier oder CSRF-Token als solche an den Browser gehen: Im Request an den Redirekt-Endpunkt müssen sie aus dem Request ermittelbar sein (z.B. im Cookie). Dafür gibt es die Konfigurations-Attribute code_verifier_value und csrf_token_value:

oauth2 "my-ac" {
  ...
  code_verifier_value = request.cookies.cv
#  csrf_token_value = request.cookies.ct
  ...
}

Ohne einen zentralen Store ist es dann auch wahrscheinlich, dass Access-/Id-/Refresh-Tokens als solche (als Cookie) im Browser gespeichert werden. Hätte man einen (von verschiedenen Couper-Instanzen verwendeten) zentralen Store, könnte man die Tokens selbst im Store speichern und nur eine Referenz darauf an den Browser gehen. Das würde ggfs. die Sicherheit noch erhöhen.
Ebenso könnten dann Code-Verifier/CSRF-Token selbst im Store verbleiben und nur Referenzen darauf an den Browser gehen.
Verbaut man sich nun durch die genannten Konfigurations-Attribute eine mögliche Erweiterung durch einen zentralen Store? Vermutlich nicht: code_verifier_value/csrf_token_value müssten dann nur anders interpretiert werden, nämlich als Referenzen auf die eigentlichen Werte. Welche Variante gewünscht ist, ließe sich sicherlich konfigurativ ausdrücken.

filex · 2021-06-16T07:38:14Z

Verbaut man sich nun durch die genannten Konfigurations-Attribute eine mögliche Erweiterung durch einen zentralen Store? Vermutlich nicht: code_verifier_value/csrf_token_value müssten dann nur anders interpretiert werden, nämlich als Referenzen auf die eigentlichen Werte. Welche Variante gewünscht ist, ließe sich sicherlich konfigurativ ausdrücken.

The current idea is, that code_verifier_value is defined as an arbitrary expression that usually involves cookies. if we would incorporate a KV interface, we could provide a lookup function as a simple solution. That would still fit the expression approach.

But I would rather make that token store a built-in feature that is configured outside of the AC. We could then reference it with a new set of config attributes that are mutually exclusive with code_verifier_value.

For the task here, I agree that we can proceed with the value-approach.

malud

First go related feedback.

accesscontrol/jwks.go

accesscontrol/oauth2.go

malud · 2021-06-16T07:38:43Z

accesscontrol/oauth2.go

+// Validate implements the AccessControl interface
+func (oa *OAuth2Callback) Validate(req *http.Request) error {
+	if req.Method != http.MethodGet {
+		return errors.Oauth2.Messagef("wrong method: %s", req.Method)


This applies to all Oauth2 errors, could we add more context e.g. oa.conf.Name as .Label(...) ?

Should the caller give the context? Or the callee?

accesscontrol/oauth2.go

malud · 2021-06-16T14:53:59Z

docs/README.md

+| `csrf`                     | <ul><li>&#9888; one of `pkce` or `csrf` is mandatory.</li><li>Use `state` or `nonce` for protection against CSRF.</li></ul> |
+| **Attributes**             | **Description** |
+| `authorization_endpoint`   | <ul><li>&#9888; Mandatory.</li><li>The authorization server endpoint URL used for authorization.</li></ul> |
+| `token_endpoint`           | <ul><li>Optional.</li><li>The authorization server endpoint URL used for requesting the token.</li></ul> |


The documentation needs to be rebased but this table should get another column for required/optional instead of html listings. Other tables will be adapted soon.

eval/lib/oauth2.go

handler/transport/oauth2.go

utils/base64url/base64url.go

eval/context.go

…llenge(); tests, documentation

Move to lib package

…r) and b) OAuth2ReqAuth (the request authorizer / token user)

… than credentials)

…rant flow" Reintroduced userinfo request for OIDC conformance tests. This reverts commit 0d1c4e7.

…smatch

malud

Some things to do, please see the comments from last week.

handler/transport/oauth2_req_auth.go

server/http_oauth2_test.go

Co-authored-by: Marcel Ludwig <marcel.ludwig@avenga.com>

…ifferent from id_token_claims

Solving Conflicts: * CHANGELOG.md * docs/REFERENCE.md * eval/context.go * eval/lib/jwt.go and adding more documentation

* Fix oauth_authorization_url function call to undefined reference * Fix missing cache setup on conf dry run * Fix missing name log value * oidc block ttl attribute is optional now; with 1h default * Change: oauth2/oidc redirect_uri attribute gets evaluated per request now * Add shutdown logAll hook entries for failed tests * Remove obsolete method #247 * Add oidc eval test * fixup obsolete assignment * Fixup possible startup errors while fetching the oidc configuration Pass uid Fixed missing update of the jwt parser if the issuer has been changed * Change memCache set/get to interface{} value * Fixup missing lock on oidc configuration refresh * Renamed CallbackURL -> RedirectURI * Ensure that redirectURI is not empty * Handling of undefinied reference in saml_sso_url() similar to beta_oauth_authorization_url() * Do not load openid configuration at start-up; only GetVerifierMethod() and GetAuthorizationEndpoint() need uid, as they are the first methods calling the openid configuration * Reference() already in config.OIDC; removed from oidc.Config * split OAuth clients code into different files * better error log messages * removed unused return values * more information in OAuth2 error messages * renamed beta_oidc.ttl -> beta_oidc.configuration_ttl * missing optional configuration_ttl does not crash Co-authored-by: Johannes Koch <johannes.koch@avenga.com>

johakoch force-pushed the oauth2-ac branch 9 times, most recently from 13c73b0 to 1a02ddb Compare May 28, 2021 07:34

johakoch force-pushed the oauth2-ac branch from 2554867 to 799014a Compare June 9, 2021 11:34

johakoch marked this pull request as ready for review June 11, 2021 12:54

malud requested changes Jun 16, 2021

View reviewed changes

johakoch force-pushed the oauth2-ac branch from bec47c5 to 69952a5 Compare June 16, 2021 17:43

malud reviewed Jun 17, 2021

View reviewed changes

eval/context.go Outdated Show resolved Hide resolved

Johannes Koch and others added 14 commits June 22, 2021 16:29

Added OAuth2 PKCE functions: oauth_code_verifier() and oauth_code_cha…

5b3debe

…llenge(); tests, documentation

Refactor: use memorize map instead of inner context

d1890b8

Move to lib package

copy memorize map when creating new Contexts

2bd1105

attach updateFunctions() to struct

3f61b3c

memoize code verifier, pass memoizing function to cty function factories

9355148

Refactored OAuth2: split functions into a) OAuth2 (the token requeste…

e06f492

…r) and b) OAuth2ReqAuth (the request authorizer / token user)

Renamed OAuth2Credentials -> OAuth2RequestConfig (meanwhile it's more…

e48c31d

… than credentials)

Fixup memorize map initialization

8191aeb

oauth2 access control

8dc904b

corrected interface comments

fc2adb3

renamed config.OAuth2 struct to config.OAuthReqAuth

bd277bb

moved config.OAuth2RA

bf1ea47

moved config.OAuth2Config to its own file

3e16172

renamed config.OAuth2Config to config.OAuth2

1abbb7f

Johannes Koch added 2 commits June 23, 2021 13:47

Revert "Removed userinfo request, not needed for authorization code g…

7e01e1e

…rant flow" Reintroduced userinfo request for OIDC conformance tests. This reverts commit 0d1c4e7.

Removed duplicate check for existing sub claim; added test for sub mi…

2376abc

…smatch

malud requested changes Jun 24, 2021

View reviewed changes

handler/transport/oauth2_req_auth.go Outdated Show resolved Hide resolved

server/http_oauth2_test.go Show resolved Hide resolved

server/http_oauth2_test.go Outdated Show resolved Hide resolved

johakoch and others added 15 commits June 24, 2021 09:16

rename function also in comment

339f7cd

Co-authored-by: Marcel Ludwig <marcel.ludwig@avenga.com>

Create and use subtest helper

56f4fe5

some more Messagef() -> Message()

0288efa

remove unnecessary comments

a2af6ba

use %q instead of '%s'

5436e22

Base64url_s256 -> Base64urlSha256

675d3d8

Context now first argument

dd39adf

Merge branch 'master' into oauth2-ac

bbfd584

Corrected indentation

f447120

Add test helper for template couper configurations

af49068

make userinfo_endpoint static

39f0bc1

Added docu for userinfo_endpoint

e4db466

make token_endpoint static; and mandatory for OAuth2 AC flow

d20a4d8

set resource server origin via templating

d0e1475

store userinfo in request.context.<label>.userinfo because it maybe d…

3495f71

…ifferent from id_token_claims

malud mentioned this pull request Jun 24, 2021

Fix: Panic in jwt_sign() function #242

Merged

Johannes Koch added 3 commits June 25, 2021 15:47

Merge branch 'master' into oauth2-ac

99c6288

Solving Conflicts: * CHANGELOG.md * docs/REFERENCE.md * eval/context.go * eval/lib/jwt.go and adding more documentation

some docu corrections/additions

8c22818

Added note about not disabling certificate validation

883addc

johakoch merged commit 0dd0d44 into master Jun 25, 2021

johakoch deleted the oauth2-ac branch June 25, 2021 17:13

malud pushed a commit that referenced this pull request Aug 12, 2021

Remove obsolete method #247

bbed896

malud pushed a commit that referenced this pull request Aug 13, 2021

Remove obsolete method #247

43d8463

malud pushed a commit that referenced this pull request Aug 17, 2021

Remove obsolete method #247

b3f192b

johakoch pushed a commit that referenced this pull request Aug 19, 2021

Remove obsolete method #247

0dcf44d

johakoch pushed a commit that referenced this pull request Aug 19, 2021

Remove obsolete method #247

cfb1d11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OAuth2 Authorization Code Grant Flow #247

OAuth2 Authorization Code Grant Flow #247

johakoch commented May 26, 2021

johakoch commented Jun 14, 2021

filex commented Jun 16, 2021

malud left a comment

malud Jun 16, 2021

johakoch Jun 24, 2021

malud Jun 16, 2021

malud left a comment

OAuth2 Authorization Code Grant Flow #247

OAuth2 Authorization Code Grant Flow #247

Conversation

johakoch commented May 26, 2021

johakoch commented Jun 14, 2021

filex commented Jun 16, 2021

malud left a comment

Choose a reason for hiding this comment

malud Jun 16, 2021

Choose a reason for hiding this comment

johakoch Jun 24, 2021

Choose a reason for hiding this comment

malud Jun 16, 2021

Choose a reason for hiding this comment

malud left a comment

Choose a reason for hiding this comment