Skip to content

Commit

Permalink
beta_oidc refinements (#284)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
Marcel Ludwig and Johannes Koch authored Aug 20, 2021
1 parent cab0670 commit 136c70d
Show file tree
Hide file tree
Showing 47 changed files with 998 additions and 642 deletions.
3 changes: 2 additions & 1 deletion accesscontrol/ac.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package accesscontrol
import (
"net/http"

"github.com/avenga/couper/config/request"
"github.com/avenga/couper/errors"
"github.com/avenga/couper/eval"
)
Expand Down Expand Up @@ -47,7 +48,7 @@ func (i ListItem) Validate(req *http.Request) error {
return errors.AccessControl.Label(i.label).Kind(i.kind).With(err)
}

if evalCtx, ok := req.Context().Value(eval.ContextType).(*eval.Context); ok {
if evalCtx, ok := req.Context().Value(request.ContextType).(*eval.Context); ok {
*req = *req.WithContext(evalCtx.WithClientRequest(req))
}

Expand Down
6 changes: 3 additions & 3 deletions accesscontrol/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ func NewOAuth2Callback(oauth2Client oauth2.AcClient) (*OAuth2Callback, error) {
// 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)
return errors.AccessControl.Messagef("wrong method: %s", req.Method)
}

_, tokenResponseData, _, err := oa.oauth2Client.GetTokenResponse(req.Context(), req.URL)
tokenResponseData, err := oa.oauth2Client.GetTokenResponse(req.Context(), req.URL)
if err != nil {
return err
return errors.AccessControl.With(err)
}

ctx := req.Context()
Expand Down
11 changes: 5 additions & 6 deletions cache/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
const maxExpiresIn = 86400

type entry struct {
value string
value interface{}
expAt int64
}

Expand Down Expand Up @@ -46,23 +46,22 @@ func (ms *MemoryStore) Del(k string) {
}

// Get return the value by the key if the ttl is not expired from the <MemoryStore>.
func (ms *MemoryStore) Get(k string) string {
func (ms *MemoryStore) Get(k string) interface{} {
ms.mu.RLock()
defer ms.mu.RUnlock()

if v, ok := ms.db[k]; ok {
if time.Now().Unix() >= v.expAt {
return ""
return nil
}

return v.value
}

return ""
return nil
}

// Set stores a key/value pair for <ttl> second(s) into the <MemoryStore>.
func (ms *MemoryStore) Set(k, v string, ttl int64) {
func (ms *MemoryStore) Set(k string, v interface{}, ttl int64) {
if ttl < 0 {
ttl = 0
} else if ttl > maxExpiresIn {
Expand Down
17 changes: 12 additions & 5 deletions cache/memory_test.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
package cache_test

import (
"context"
"testing"
"time"

"github.com/avenga/couper/cache"
"github.com/avenga/couper/internal/test"
)

func TestCache_All(t *testing.T) {
ms := cache.New(nil, nil)
log, _ := test.NewLogger()
logger := log.WithContext(context.Background())

if v := ms.Get("key"); v != "" {
t.Errorf("Empty string expected, given %q", v)
quitCh := make(chan struct{})
defer close(quitCh)
ms := cache.New(logger, quitCh)

if v := ms.Get("key"); v != nil {
t.Errorf("Nil expected, given %q", v)
}

go ms.Set("key", "val", 2)
Expand All @@ -31,8 +38,8 @@ func TestCache_All(t *testing.T) {

time.Sleep(1700 * time.Millisecond)

if v := ms.Get("key"); v != "" {
t.Errorf("Empty string expected, given %q", v)
if v := ms.Get("key"); v != nil {
t.Errorf("Nil expected, given %q", v)
}
if v := ms.Get("del"); v != "del" {
t.Errorf("Expected 'del', given %q", v)
Expand Down
13 changes: 5 additions & 8 deletions config/ac_oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ var _ OAuth2AcClient = &OAuth2AC{}
var _ OAuth2AcAS = &OAuth2AC{}
var _ OAuth2Authorization = &OAuth2AC{}

// OAuth2AC represents represents an oauth2 block for an OAuth2 client using the authorization code flow.
// OAuth2AC represents an oauth2 block for an OAuth2 client using the authorization code flow.
type OAuth2AC struct {
AccessControlSetter
AuthorizationEndpoint string `hcl:"authorization_endpoint"`
Expand All @@ -18,12 +18,12 @@ type OAuth2AC struct {
ClientSecret string `hcl:"client_secret"`
GrantType string `hcl:"grant_type"`
Name string `hcl:"name,label"`
RedirectURI string `hcl:"redirect_uri"`
Remain hcl.Body `hcl:",remain"`
Scope *string `hcl:"scope,optional"`
TokenEndpoint string `hcl:"token_endpoint"`
TokenEndpointAuthMethod *string `hcl:"token_endpoint_auth_method,optional"`
VerifierMethod string `hcl:"verifier_method"`

// internally used
Backend hcl.Body
BodyContent *hcl.BodyContent
Expand All @@ -49,6 +49,7 @@ func (oa OAuth2AC) Schema(inline bool) *hcl.BodySchema {

type Inline struct {
Backend *Backend `hcl:"backend,block"`
RedirectURI string `hcl:"redirect_uri"`
VerifierValue string `hcl:"verifier_value"`
}

Expand Down Expand Up @@ -85,11 +86,7 @@ func (oa OAuth2AC) GetScope() string {
return *oa.Scope
}

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

func (oa OAuth2AC) GetAuthorizationEndpoint() (string, error) {
func (oa OAuth2AC) GetAuthorizationEndpoint(_ string) (string, error) {
return oa.AuthorizationEndpoint, nil
}

Expand All @@ -102,6 +99,6 @@ func (oa OAuth2AC) GetTokenEndpointAuthMethod() *string {
}

// GetVerifierMethod retrieves the verifier method (ccm_s256 or state)
func (oa OAuth2AC) GetVerifierMethod() (string, error) {
func (oa OAuth2AC) GetVerifierMethod(_ string) (string, error) {
return oa.VerifierMethod, nil
}
11 changes: 5 additions & 6 deletions config/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type OAuth2AS interface {
// OAuth2AcAS represents the authorization server configuration for OAuth2 clients using the authorization code flow.
type OAuth2AcAS interface {
OAuth2AS
GetAuthorizationEndpoint() (string, error)
GetAuthorizationEndpoint(uid string) (string, error)
}

// OidcAS represents the OIDC server configuration for OIDC clients.
Expand All @@ -41,18 +41,17 @@ type OAuth2Client interface {
type OAuth2AcClient interface {
OAuth2Client
GetName() string
GetRedirectURI() string
// GetVerifierMethod retrieves the verifier method (ccm_s256, nonce or state)
GetVerifierMethod() (string, error)
GetVerifierMethod(uid string) (string, error)
GetBodyContent() *hcl.BodyContent
}

// OAuth2Authorization represents the configuration for the OAuth2 authorization URL function
type OAuth2Authorization interface {
GetAuthorizationEndpoint() (string, error)
Inline
GetAuthorizationEndpoint(uid string) (string, error)
GetClientID() string
GetName() string
GetRedirectURI() string
GetScope() string
GetVerifierMethod() (string, error)
GetVerifierMethod(uid string) (string, error)
}
9 changes: 3 additions & 6 deletions config/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ type OIDC struct {
ClientSecret string `hcl:"client_secret"`
ConfigurationURL string `hcl:"configuration_url"`
Name string `hcl:"name,label"`
RedirectURI string `hcl:"redirect_uri"`
Remain hcl.Body `hcl:",remain"`
Scope *string `hcl:"scope,optional"`
TokenEndpointAuthMethod *string `hcl:"token_endpoint_auth_method,optional"`
TTL string `hcl:"ttl"`
ConfigurationTTL string `hcl:"configuration_ttl,optional"`
VerifierMethod string `hcl:"verifier_method,optional"`

// internally used
Backend hcl.Body
BodyContent *hcl.BodyContent
Expand All @@ -44,6 +44,7 @@ func (o OIDC) Schema(inline bool) *hcl.BodySchema {

type Inline struct {
Backend *Backend `hcl:"backend,block"`
RedirectURI string `hcl:"redirect_uri"`
VerifierValue string `hcl:"verifier_value"`
}

Expand Down Expand Up @@ -80,10 +81,6 @@ func (o OIDC) GetScope() string {
return "openid " + *o.Scope
}

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

func (o OIDC) GetTokenEndpointAuthMethod() *string {
return o.TokenEndpointAuthMethod
}
3 changes: 2 additions & 1 deletion config/request/context_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package request
type ContextKey uint8

const (
UID ContextKey = iota
ContextType ContextKey = iota
AccessControls
BackendName
Endpoint
Expand All @@ -17,6 +17,7 @@ const (
ServerName
TokenRequest
TokenRequestRetries
UID
URLAttribute
Wildcard
XFF
Expand Down
31 changes: 21 additions & 10 deletions config/runtime/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/avenga/couper/cache"
"github.com/avenga/couper/config"
"github.com/avenga/couper/config/reader"
"github.com/avenga/couper/config/request"
"github.com/avenga/couper/config/runtime/server"
"github.com/avenga/couper/errors"
"github.com/avenga/couper/eval"
Expand Down Expand Up @@ -92,25 +93,25 @@ func NewServerConfiguration(conf *config.Couper, log *logrus.Entry, memStore *ca
noopReq, _ := http.NewRequest(http.MethodGet, "https://couper.io", nil)
noopResp := httptest.NewRecorder().Result()
noopResp.Request = noopReq
evalContext := conf.Context.Value(eval.ContextType).(*eval.Context)
evalContext := conf.Context.Value(request.ContextType).(*eval.Context)
confCtx := evalContext.WithClientRequest(noopReq).WithBeresps(noopResp).HCLContext()

oidcConfigs, ocErr := configureOidcConfigs(conf, confCtx, log, memStore)
if ocErr != nil {
return nil, ocErr
}
evalContext = evalContext.WithOidcConfig(oidcConfigs)
evalContext.WithOidcConfig(oidcConfigs)

accessControls, acErr := configureAccessControls(conf, confCtx, log, memStore, oidcConfigs)
if acErr != nil {
return nil, acErr
}

var (
serverConfiguration ServerConfiguration = make(ServerConfiguration)
defaultPort int = conf.Settings.DefaultPort
endpointHandlers endpointHandler = make(endpointHandler)
isHostsMandatory bool = len(conf.Servers) > 1
serverConfiguration = make(ServerConfiguration)
defaultPort = conf.Settings.DefaultPort
endpointHandlers = make(endpointHandler)
isHostsMandatory = len(conf.Servers) > 1
)

for _, srvConf := range conf.Servers {
Expand Down Expand Up @@ -410,8 +411,8 @@ func whichCORS(parent *config.Server, this interface{}) *config.CORS {
return corsData
}

func configureOidcConfigs(conf *config.Couper, confCtx *hcl.EvalContext, log *logrus.Entry, memStore *cache.MemoryStore) (map[string]*oidc.OidcConfig, error) {
oidcConfigs := make(map[string]*oidc.OidcConfig)
func configureOidcConfigs(conf *config.Couper, confCtx *hcl.EvalContext, log *logrus.Entry, memStore *cache.MemoryStore) (oidc.Configs, error) {
oidcConfigs := make(oidc.Configs)
if conf.Definitions != nil {
for _, oidcConf := range conf.Definitions.OIDC {
confErr := errors.Configuration.Label(oidcConf.Name)
Expand All @@ -420,7 +421,7 @@ func configureOidcConfigs(conf *config.Couper, confCtx *hcl.EvalContext, log *lo
return nil, confErr.With(err)
}

oidcConfig, err := oidc.NewOidcConfig(oidcConf, backend, memStore)
oidcConfig, err := oidc.NewConfig(oidcConf, backend, memStore)
if err != nil {
return nil, confErr.With(err)
}
Expand All @@ -432,7 +433,9 @@ func configureOidcConfigs(conf *config.Couper, confCtx *hcl.EvalContext, log *lo
return oidcConfigs, nil
}

func configureAccessControls(conf *config.Couper, confCtx *hcl.EvalContext, log *logrus.Entry, memStore *cache.MemoryStore, oidcConfigs map[string]*oidc.OidcConfig) (ACDefinitions, error) {
func configureAccessControls(conf *config.Couper, confCtx *hcl.EvalContext, log *logrus.Entry,
memStore *cache.MemoryStore, oidcConfigs oidc.Configs) (ACDefinitions, error) {

accessControls := make(ACDefinitions)

if conf.Definitions != nil {
Expand Down Expand Up @@ -527,6 +530,14 @@ func configureAccessControls(conf *config.Couper, confCtx *hcl.EvalContext, log
return nil, confErr.With(err)
}

if oidcConfig.VerifierMethod != "" &&
oidcConfig.VerifierMethod != config.CcmS256 &&
oidcConfig.VerifierMethod != "nonce" {
return nil, errors.Configuration.
Label(oidcConf.Name).
Messagef("verifier_method %s not supported", oidcConfig.VerifierMethod)
}

oa, err := ac.NewOAuth2Callback(oidcClient)
if err != nil {
return nil, confErr.With(err)
Expand Down
2 changes: 1 addition & 1 deletion docs/REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ Like all [Access Control](#access-control) types, the `beta_oidc` block is defin
| :------------------------------ | :--------------- | :--------------- | :--------------- | :--------------- | :--------------- |
| `backend` |string|-|[Backend Block Reference](#backend-block)| &#9888; Do not disable the peer certificate validation with `disable_certificate_validation = true`! |-|
| `configuration_url` | string |-| The OpenID configuration URL. |&#9888; required|-|
| `ttl` | duration |-| The duration to cache the OpenID configuration located at `configuration_url`. |&#9888; required| `ttl = "1d"` |
| `configuration_ttl` | duration | `1h` | The duration to cache the OpenID configuration located at `configuration_url`. | - | `configuration_ttl = "1d"` |
| `token_endpoint_auth_method` |string|`client_secret_basic`|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.|-|
| `redirect_uri` | string |-| The Couper endpoint for receiving the authorization code. |&#9888; required. Relative URL references are resolved against the origin of the current request URL.|-|
| `client_id`| string|-|The client identifier.|&#9888; required|-|
Expand Down
1 change: 1 addition & 0 deletions errors/couper.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ var (
ClientRequest = &Error{synopsis: "client request error", httpStatus: http.StatusBadRequest}
Evaluation = &Error{synopsis: "expression evaluation error", kinds: []string{"evaluation"}, httpStatus: http.StatusInternalServerError}
Configuration = &Error{synopsis: "configuration error", kinds: []string{"configuration"}, httpStatus: http.StatusInternalServerError}
OAuth2 = &Error{synopsis: "oauth2 error", httpStatus: http.StatusBadRequest}
Proxy = &Error{synopsis: "proxy error", httpStatus: http.StatusBadGateway}
Request = &Error{synopsis: "request error", httpStatus: http.StatusBadGateway}
RouteNotFound = &Error{synopsis: "route not found error", httpStatus: http.StatusNotFound}
Expand Down
2 changes: 1 addition & 1 deletion errors/type_defintions.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ var Definitions = []*Error{

AccessControl.Kind("saml2"),

AccessControl.Kind("oauth2"),
OAuth2.Kind("oauth2"),
}
44 changes: 44 additions & 0 deletions eval/content/attribute.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package content

import (
"context"

"github.com/hashicorp/hcl/v2"

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

type Context interface {
HCLContext() *hcl.EvalContext
}

func GetContextAttribute(context hcl.Body, httpContext context.Context, name string) (string, error) {
ctx, ok := httpContext.Value(request.ContextType).(Context)
if !ok {
return "", nil
}
evalCtx := ctx.HCLContext()

schema := &hcl.BodySchema{Attributes: []hcl.AttributeSchema{{Name: name}}}
content, _, _ := context.PartialContent(schema)
if content == nil || len(content.Attributes) == 0 {
return "", nil
}

return GetAttribute(evalCtx, content, name)
}

func GetAttribute(ctx *hcl.EvalContext, content *hcl.BodyContent, name string) (string, error) {
attr := content.Attributes
if _, ok := attr[name]; !ok {
return "", nil
}

val, diags := attr[name].Expr.Value(ctx)
if diags.HasErrors() {
return "", diags
}

return seetie.ValueToString(val), nil
}
Loading

0 comments on commit 136c70d

Please sign in to comment.