Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make redirect_uri more static #589

Merged
merged 7 commits into from
Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

Unreleased changes are available as `avenga/couper:edge` container.

* **Fixed**
* Aligned the evaluation of `beta_oauth2`/`oidc` `redirect_uri` to `saml` `sp_acs_url` ([#589](https://github.com/avenga/couper/pull/589))

---

## [1.10.0](https://github.com/avenga/couper/releases/tag/v1.10.0)
Expand Down
6 changes: 5 additions & 1 deletion config/ac_oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type OAuth2AC struct {
ClientSecret string `hcl:"client_secret" docs:"The client password."`
GrantType string `hcl:"grant_type" docs:"The grant type. Required, to be set to: {authorization_code}"`
Name string `hcl:"name,label"`
RedirectURI string `hcl:"redirect_uri" docs:"The Couper endpoint for receiving the authorization code. Relative URL references are resolved against the origin of the current request URL. The origin can be changed with the {accept_forwarded_url}([settings](settings)) attribute if Couper is running behind a proxy."`
Remain hcl.Body `hcl:",remain"`
Scope *string `hcl:"scope,optional" docs:"A space separated list of requested scope values for the access token."`
TokenEndpoint string `hcl:"token_endpoint" docs:"The authorization server endpoint URL used for requesting the token."`
Expand Down Expand Up @@ -56,7 +57,6 @@ func (oa *OAuth2AC) Inline() interface{} {
type Inline struct {
meta.LogFieldsAttribute
Backend *Backend `hcl:"backend,block"`
RedirectURI string `hcl:"redirect_uri" docs:"The Couper endpoint for receiving the authorization code. Relative URL references are resolved against the origin of the current request URL. The origin can be changed with the {accept_forwarded_url}([settings](settings)) attribute if Couper is running behind a proxy."`
VerifierValue string `hcl:"verifier_value" docs:"The value of the (unhashed) verifier. E.g. using cookie value created with {oauth2_verifier()} function](../functions)"`
}

Expand Down Expand Up @@ -96,6 +96,10 @@ func (oa *OAuth2AC) GetGrantType() string {
return oa.GrantType
}

func (oa *OAuth2AC) GetRedirectURI() string {
return oa.RedirectURI
}

func (oa *OAuth2AC) GetScope() string {
if oa.Scope == nil {
return ""
Expand Down
6 changes: 5 additions & 1 deletion config/ac_oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type OIDC struct {
JWKsMaxStale string `hcl:"jwks_max_stale,optional" docs:"Time period the cached JWK set stays valid after its TTL has passed." type:"duration" default:"1h"`
Name string `hcl:"name,label"`
Remain hcl.Body `hcl:",remain"`
RedirectURI string `hcl:"redirect_uri" docs:"The Couper endpoint for receiving the authorization code. Relative URL references are resolved against the origin of the current request URL. The origin can be changed with the {accept_forwarded_url}({settings} block) attribute if Couper is running behind a proxy."`
Scope *string `hcl:"scope,optional" docs:"A space separated list of requested scope values for the access token."`
TokenEndpointAuthMethod *string `hcl:"token_endpoint_auth_method,optional" docs:"Defines the method to authenticate the client at the token endpoint. If set to {client_secret_post}, the client credentials are transported in the request body. If set to {client_secret_basic}, the client credentials are transported via Basic Authentication." default:"client_secret_basic"`
ConfigurationTTL string `hcl:"configuration_ttl,optional" docs:"The duration to cache the OpenID configuration located at {configuration_url}." type:"duration" default:"1h"`
Expand Down Expand Up @@ -73,7 +74,6 @@ func (o *OIDC) Inline() interface{} {
type Inline struct {
meta.LogFieldsAttribute
Backend *Backend `hcl:"backend,block"`
RedirectURI string `hcl:"redirect_uri" docs:"The Couper endpoint for receiving the authorization code. Relative URL references are resolved against the origin of the current request URL. The origin can be changed with the {accept_forwarded_url}({settings} block) attribute if Couper is running behind a proxy."`
VerifierValue string `hcl:"verifier_value" docs:"The value of the (unhashed) verifier."`

AuthorizationBackend *Backend `hcl:"authorization_backend,block"`
Expand Down Expand Up @@ -121,6 +121,10 @@ func (o *OIDC) GetGrantType() string {
return "authorization_code"
}

func (o *OIDC) GetRedirectURI() string {
return o.RedirectURI
}

func (o *OIDC) GetScope() string {
if o.Scope == nil {
return "openid"
Expand Down
2 changes: 2 additions & 0 deletions config/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type OAuth2Client interface {
type OAuth2AcClient interface {
OAuth2Client
GetGrantType() string
GetRedirectURI() string
// GetVerifierMethod retrieves the verifier method (ccm_s256, nonce or state)
GetVerifierMethod() (string, error)
}
Expand All @@ -31,6 +32,7 @@ type OAuth2Authorization interface {
Inline
GetAuthorizationEndpoint() (string, error)
GetClientID() string
GetRedirectURI() string
GetScope() string
GetVerifierMethod() (string, error)
}
3 changes: 0 additions & 3 deletions eval/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,6 @@ func (c *Context) WithBeresp(beresp *http.Response, readBody bool) *Context {
mergeBackendVariables(ctx.eval, BackendRequests, bereqs)
mergeBackendVariables(ctx.eval, BackendResponses, resps)

clientOrigin, _ := seetie.ValueToMap(ctx.eval.Variables[ClientRequest])[Origin].(string)
originURL, _ := url.Parse(clientOrigin)
ctx.updateRequestRelatedFunctions(originURL)
ctx.updateFunctions()

return ctx
Expand Down
20 changes: 2 additions & 18 deletions eval/lib/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,9 @@ import (
"github.com/zclconf/go-cty/cty/function"

"github.com/avenga/couper/config"
"github.com/avenga/couper/internal/seetie"
)

const (
RedirectURI = "redirect_uri"
CodeVerifier = "code_verifier"
FnOAuthAuthorizationURL = "oauth2_authorization_url"
FnOAuthVerifier = "oauth2_verifier"
Expand Down Expand Up @@ -52,23 +50,9 @@ func NewOAuthAuthorizationURLFunction(ctx *hcl.EvalContext, oauth2s map[string]c
return emptyStringVal, err
}

body := oauth2.HCLBody()
bodyContent, _, diags := body.PartialContent(oauth2.Schema(true))
if diags.HasErrors() {
return emptyStringVal, diags
}

var redirectURI string
if attr, ok := bodyContent.Attributes[RedirectURI]; ok {
val, verr := evalFn(ctx, attr.Expr)
if verr != nil {
return emptyStringVal, verr
}
redirectURI = seetie.ValueToString(val)
}

redirectURI := oauth2.GetRedirectURI()
if redirectURI == "" {
return emptyStringVal, fmt.Errorf("%s is required", RedirectURI)
return emptyStringVal, fmt.Errorf("redirect_uri is required")
}

absRedirectURI, err := AbsoluteURL(redirectURI, origin)
Expand Down
21 changes: 8 additions & 13 deletions eval/lib/oauth2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,15 @@ definitions {
client_id = "test-id"
client_secret = "test-s3cr3t"
configuration_url = "` + u + `"
redirect_uri = "${request.headers.x-want},${backend_requests.default.headers.x-want},${backend_responses.default.headers.x-want}"
redirect_uri = split(" ", env.REDIR_URIS)[0]
verifier_value = "asdf"
}
}
defaults {
environment_variables = {
REDIR_URIS = "/cb /cb2"
}
}
`

log, _ := test.NewLogger()
Expand All @@ -76,23 +81,12 @@ definitions {
_, err = runtime.NewServerConfiguration(couperConf, logger, memStore)
helper.Must(err)

// redirect_uri = "${request.headers.x-want},${backend_requests.default.headers.x-want},${backend_responses.default.headers.x-want}"
want := "https://couper.io/cb,https://couper.io/cb,https://couper.io/cb"

req, rerr := http.NewRequest(http.MethodGet, "https://couper.io/", nil)
helper.Must(rerr)
req = req.Clone(context.Background())
req.Header.Set("x-want", "https://couper.io/cb")

res := &http.Response{
Header: http.Header{"x-want": []string{"https://couper.io/cb"}},
Request: req,
StatusCode: http.StatusNoContent,
}

hclCtx := couperConf.Context.(*eval.Context).
WithClientRequest(req).
WithBeresp(res, false).
HCLContext()

val, furr := hclCtx.Functions[lib.FnOAuthAuthorizationURL].Call([]cty.Value{cty.StringVal("auth-ref")})
Expand All @@ -102,7 +96,8 @@ definitions {
authURLObj, perr := url.Parse(authURL)
helper.Must(perr)

if value := authURLObj.Query().Get(lib.RedirectURI); value != want {
want := "https://couper.io/cb"
if value := authURLObj.Query().Get("redirect_uri"); value != want {
t.Errorf("Want: %v; got: %v", want, value)
}
}
14 changes: 4 additions & 10 deletions oauth2/abstract_auth_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,9 @@ func (a AbstractAuthCodeClient) ExchangeCodeAndGetTokenResponse(req *http.Reques
return nil, errors.Oauth2.Messagef("missing code query parameter; query=%q", callbackURL.RawQuery)
}

ctx := eval.ContextFromRequest(req).HCLContext()

redirectURIVal, err := eval.ValueFromBodyAttribute(ctx, a.clientConfig.HCLBody(), lib.RedirectURI)
if err != nil {
return nil, errors.Oauth2.With(err)
}

redirectURI := seetie.ValueToString(redirectURIVal)
redirectURI := a.acClientConf.GetRedirectURI()
if redirectURI == "" {
return nil, errors.Oauth2.With(err).Messagef("%s is required", lib.RedirectURI)
return nil, errors.Oauth2.Message("redirect_uri is required")
}

absoluteURL, err := lib.AbsoluteURL(redirectURI, eval.NewRawOrigin(callbackURL))
Expand All @@ -50,14 +43,15 @@ func (a AbstractAuthCodeClient) ExchangeCodeAndGetTokenResponse(req *http.Reques
"redirect_uri": {absoluteURL},
}

ctx := eval.ContextFromRequest(req).HCLContext()
verifierVal, err := eval.ValueFromBodyAttribute(ctx, a.clientConfig.HCLBody(), "verifier_value")
if err != nil {
return nil, errors.Oauth2.With(err)
}

verifierValue := strings.TrimSpace(seetie.ValueToString(verifierVal))
if verifierValue == "" {
return nil, errors.Oauth2.With(err).Messagef("Empty verifier_value")
return nil, errors.Oauth2.With(err).Message("Empty verifier_value")
}

verifierMethod, err := a.acClientConf.GetVerifierMethod()
Expand Down