cim/zitadel-oidc
5
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: handle single aud string claim, extract en/decoder interface, comments (#51)

Browse source 
* en/decoding abstraction

* some comments

* fix token validation and error messages

* fix: audience mapping (single aud string)

* fix tests with VerifyIdToken

* reformat imports

* go mod tidy

* Update pkg/oidc/authorization.go

Co-authored-by: Silvan <silvan.reusser@gmail.com>

* Update pkg/oidc/authorization.go

Co-authored-by: Silvan <silvan.reusser@gmail.com>

* Update pkg/op/authrequest_test.go

Co-authored-by: Silvan <silvan.reusser@gmail.com>

* fix capitalization

Co-authored-by: Silvan <silvan.reusser@gmail.com>
This commit is contained in:
Livio Amstutz 2020-09-07 12:32:35 +02:00 • committed by GitHub
parent 822ffb581f
commit abd3b6f521
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
24 changed files with 381 additions and 139 deletions
Download patch file Download diff file Expand all files Collapse all files

48
pkg/op/authrequest.go
View file

@ -2,13 +2,11 @@ package op
import (
"context"
"errors"
"fmt"
"net/http"
"strings"
"github.com/gorilla/mux"
"github.com/gorilla/schema"
"github.com/caos/oidc/pkg/oidc"
"github.com/caos/oidc/pkg/rp"
@ -17,33 +15,37 @@ import (
type Authorizer interface {
Storage() Storage
Decoder() *schema.Decoder
Encoder() *schema.Encoder
Decoder() utils.Decoder
Encoder() utils.Encoder
Signer() Signer
IDTokenVerifier() rp.Verifier
Crypto() Crypto
Issuer() string
}
type ValidationAuthorizer interface {
//AuthorizeValidator is an extension of Authorizer interface
//implementing it's own validation mechanism for the auth request
type AuthorizeValidator interface {
Authorizer
ValidateAuthRequest(context.Context, *oidc.AuthRequest, Storage, rp.Verifier) (string, error)
}
//ValidationAuthorizer is an extension of Authorizer interface
//implementing it's own validation mechanism for the auth request
//
//Deprecated: ValidationAuthorizer exists for historical compatibility. Use ValidationAuthorizer itself
type ValidationAuthorizer AuthorizeValidator
//Authorize handles the authorization request, including
//parsing, validating, storing and finally redirecting to the login handler
func Authorize(w http.ResponseWriter, r *http.Request, authorizer Authorizer) {
err := r.ParseForm()
authReq, err := ParseAuthorizeRequest(r, authorizer.Decoder())
if err != nil {
AuthRequestError(w, r, nil, ErrInvalidRequest("cannot parse form"), authorizer.Encoder())
return
}
authReq := new(oidc.AuthRequest)
err = authorizer.Decoder().Decode(authReq, r.Form)
if err != nil {
AuthRequestError(w, r, nil, ErrInvalidRequest(fmt.Sprintf("cannot parse auth request: %v", err)), authorizer.Encoder())
AuthRequestError(w, r, authReq, err, authorizer.Encoder())
return
}
validation := ValidateAuthRequest
if validater, ok := authorizer.(ValidationAuthorizer); ok {
if validater, ok := authorizer.(AuthorizeValidator); ok {
validation = validater.ValidateAuthRequest
}
userID, err := validation(r.Context(), authReq, authorizer.Storage(), authorizer.IDTokenVerifier())
@ -64,6 +66,19 @@ func Authorize(w http.ResponseWriter, r *http.Request, authorizer Authorizer) {
RedirectToLogin(req.GetID(), client, w, r)
}
func ParseAuthorizeRequest(r *http.Request, decoder utils.Decoder) (*oidc.AuthRequest, error) {
err := r.ParseForm()
if err != nil {
return nil, ErrInvalidRequest("cannot parse form")
}
authReq := new(oidc.AuthRequest)
err = decoder.Decode(authReq, r.Form)
if err != nil {
return nil, ErrInvalidRequest(fmt.Sprintf("cannot parse auth request: %v", err))
}
return authReq, nil
}
func ValidateAuthRequest(ctx context.Context, authReq *oidc.AuthRequest, storage Storage, verifier rp.Verifier) (string, error) {
client, err := storage.GetClientByClientID(ctx, authReq.ClientID)
if err != nil {
@ -95,7 +110,6 @@ func ValidateAuthReqRedirectURI(client Client, uri string, responseType oidc.Res
if uri == "" {
return ErrInvalidRequestRedirectURI("The redirect_uri is missing in the request. Please ensure it is added to the request. If you have any questions, you may contact the administrator of the application.")
}
if !utils.Contains(client.RedirectURIs(), uri) {
return ErrInvalidRequestRedirectURI("The requested redirect_uri is missing in the client configuration. If you have any questions, you may contact the administrator of the application.")
}
@ -138,7 +152,7 @@ func ValidateAuthReqIDTokenHint(ctx context.Context, idTokenHint string, verifie
if idTokenHint == "" {
return "", nil
}
claims, err := verifier.Verify(ctx, "", idTokenHint)
claims, err := verifier.VerifyIDToken(ctx, idTokenHint)
if err != nil {
return "", ErrInvalidRequest("The id_token_hint is invalid. If you have any questions, you may contact the administrator of the application.")
}
@ -160,7 +174,7 @@ func AuthorizeCallback(w http.ResponseWriter, r *http.Request, authorizer Author
return
}
if !authReq.Done() {
AuthRequestError(w, r, authReq, errors.New("user not logged in"), authorizer.Encoder())
AuthRequestError(w, r, authReq, ErrInteractionRequired("Unfortunately, the user may is not logged in and/or additional interaction is required."), authorizer.Encoder())
return
}
AuthResponse(authReq, authorizer, w, r)

138
pkg/op/authrequest_test.go
View file

@ -3,66 +3,140 @@ package op_test
import (
"net/http"
"net/http/httptest"
"strings"
"net/url"
"reflect"
"testing"
"github.com/gorilla/schema"
"github.com/stretchr/testify/require"
"github.com/caos/oidc/pkg/oidc"
"github.com/caos/oidc/pkg/op"
"github.com/caos/oidc/pkg/op/mock"
"github.com/caos/oidc/pkg/rp"
"github.com/caos/oidc/pkg/utils"
)
func TestAuthorize(t *testing.T) {
// testCallback := func(t *testing.T, clienID string) callbackHandler {
// return func(authReq *oidc.AuthRequest, client oidc.Client, w http.ResponseWriter, r *http.Request) {
// // require.Equal(t, clientID, client.)
// }
// }
// testErr := func(t *testing.T, expected error) errorHandler {
// return func(w http.ResponseWriter, r *http.Request, authReq *oidc.AuthRequest, err error) {
// require.Equal(t, expected, err)
// }
// }
//
//TOOD: tests will be implemented in branch for service accounts
//func TestAuthorize(t *testing.T) {
// // testCallback := func(t *testing.T, clienID string) callbackHandler {
// // return func(authReq *oidc.AuthRequest, client oidc.Client, w http.ResponseWriter, r *http.Request) {
// // // require.Equal(t, clientID, client.)
// // }
// // }
// // testErr := func(t *testing.T, expected error) errorHandler {
// // return func(w http.ResponseWriter, r *http.Request, authReq *oidc.AuthRequest, err error) {
// // require.Equal(t, expected, err)
// // }
// // }
// type args struct {
// w http.ResponseWriter
// r *http.Request
// authorizer op.Authorizer
// }
// tests := []struct {
// name string
// args args
// }{
// {
// "parsing fails",
// args{
// httptest.NewRecorder(),
// &http.Request{Method: "POST", Body: nil},
// mock.NewAuthorizerExpectValid(t, true),
// // testCallback(t, ""),
// // testErr(t, ErrInvalidRequest("cannot parse form")),
// },
// },
// {
// "decoding fails",
// args{
// httptest.NewRecorder(),
// func() *http.Request {
// r := httptest.NewRequest("POST", "/authorize", strings.NewReader("client_id=foo"))
// r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
// return r
// }(),
// mock.NewAuthorizerExpectValid(t, true),
// // testCallback(t, ""),
// // testErr(t, ErrInvalidRequest("cannot parse auth request")),
// },
// },
// // {"decoding fails", args{httptest.NewRecorder(), &http.Request{}, mock.NewAuthorizerExpectValid(t), nil, testErr(t, nil)}},
// }
// for _, tt := range tests {
// t.Run(tt.name, func(t *testing.T) {
// op.Authorize(tt.args.w, tt.args.r, tt.args.authorizer)
// })
// }
//}
func TestParseAuthorizeRequest(t *testing.T) {
type args struct {
w http.ResponseWriter
r *http.Request
authorizer op.Authorizer
r *http.Request
decoder utils.Decoder
}
type res struct {
want *oidc.AuthRequest
err bool
}
tests := []struct {
name string
args args
res res
}{
{
"parsing fails",
"parsing form error",
args{
httptest.NewRecorder(),
&http.Request{Method: "POST", Body: nil},
mock.NewAuthorizerExpectValid(t, true),
// testCallback(t, ""),
// testErr(t, ErrInvalidRequest("cannot parse form")),
&http.Request{URL: &url.URL{RawQuery: "invalid=%%param"}},
schema.NewDecoder(),
},
res{
nil,
true,
},
},
{
"decoding fails",
"decoding error",
args{
httptest.NewRecorder(),
func() *http.Request {
r := httptest.NewRequest("POST", "/authorize", strings.NewReader("client_id=foo"))
r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
return r
&http.Request{URL: &url.URL{RawQuery: "unknown=value"}},
func() utils.Decoder {
decoder := schema.NewDecoder()
decoder.IgnoreUnknownKeys(false)
return decoder
}(),
mock.NewAuthorizerExpectValid(t, true),
// testCallback(t, ""),
// testErr(t, ErrInvalidRequest("cannot parse auth request")),
},
res{
nil,
true,
},
},
{
"parsing ok",
args{
&http.Request{URL: &url.URL{RawQuery: "scope=openid"}},
func() utils.Decoder {
decoder := schema.NewDecoder()
decoder.IgnoreUnknownKeys(false)
return decoder
}(),
},
res{
&oidc.AuthRequest{Scopes: oidc.Scopes{"openid"}},
false,
},
},
// {"decoding fails", args{httptest.NewRecorder(), &http.Request{}, mock.NewAuthorizerExpectValid(t), nil, testErr(t, nil)}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
op.Authorize(tt.args.w, tt.args.r, tt.args.authorizer)
got, err := op.ParseAuthorizeRequest(tt.args.r, tt.args.decoder)
if (err != nil) != tt.res.err {
t.Errorf("ParseAuthorizeRequest() error = %v, wantErr %v", err, tt.res.err)
}
if !reflect.DeepEqual(got, tt.res.want) {
t.Errorf("ParseAuthorizeRequest() got = %v, want %v", got, tt.res.want)
}
})
}
}

3
pkg/op/client.go
View file

@ -1,8 +1,9 @@
package op
import (
"github.com/caos/oidc/pkg/oidc"
"time"
"github.com/caos/oidc/pkg/oidc"
)
const (

9
pkg/op/config_test.go
View file

@ -1,8 +1,9 @@
package op
import "testing"
import "os"
import (
"os"
"testing"
)
func TestValidateIssuer(t *testing.T) {
type args struct {
@ -78,7 +79,7 @@ func TestValidateIssuerDevLocalAllowed(t *testing.T) {
wantErr bool
}{
{
"localhost with http ok",
"localhost with http with dev ok",
args{"http://localhost:9999"},
false,
},

5
pkg/op/default_op.go
View file

@ -13,6 +13,7 @@ import (
"github.com/caos/oidc/pkg/oidc"
"github.com/caos/oidc/pkg/rp"
"github.com/caos/oidc/pkg/utils"
)
const (
@ -254,11 +255,11 @@ func (p *DefaultOP) VerifySignature(ctx context.Context, jws *jose.JSONWebSignat
return payload, err
}
func (p *DefaultOP) Decoder() *schema.Decoder {
func (p *DefaultOP) Decoder() utils.Decoder {
return p.decoder
}
func (p *DefaultOP) Encoder() *schema.Encoder {
func (p *DefaultOP) Encoder() utils.Encoder {
return p.encoder
}

11
pkg/op/discovery_test.go
View file

@ -6,12 +6,13 @@ import (
"reflect"
"testing"
"github.com/caos/oidc/pkg/oidc"
"github.com/caos/oidc/pkg/op"
"github.com/caos/oidc/pkg/op/mock"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"
"gopkg.in/square/go-jose.v2"
"github.com/caos/oidc/pkg/oidc"
"github.com/caos/oidc/pkg/op"
"github.com/caos/oidc/pkg/op/mock"
)
func TestDiscover(t *testing.T) {
@ -147,7 +148,7 @@ func TestSupportedClaims(t *testing.T) {
}
func Test_SigAlgorithms(t *testing.T) {
m := mock.NewMockSigner(gomock.NewController((t)))
m := mock.NewMockSigner(gomock.NewController(t))
type args struct {
s op.Signer
}
@ -199,7 +200,7 @@ func Test_SubjectTypes(t *testing.T) {
}
func Test_AuthMethods(t *testing.T) {
m := mock.NewMockConfiguration(gomock.NewController((t)))
m := mock.NewMockConfiguration(gomock.NewController(t))
type args struct {
c op.Configuration
}

26
pkg/op/error.go
View file

@ -4,15 +4,15 @@ import (
"fmt"
"net/http"
"github.com/gorilla/schema"
"github.com/caos/oidc/pkg/oidc"
"github.com/caos/oidc/pkg/utils"
)
const (
InvalidRequest errorType = "invalid_request"
ServerError errorType = "server_error"
InvalidRequest errorType = "invalid_request"
InvalidRequestURI errorType = "invalid_request_uri"
InteractionRequired errorType = "interaction_required"
ServerError errorType = "server_error"
)
var (
@ -24,11 +24,17 @@ var (
}
ErrInvalidRequestRedirectURI = func(description string) *OAuthError {
return &OAuthError{
ErrorType: InvalidRequest,
ErrorType: InvalidRequestURI,
Description: description,
redirectDisabled: true,
}
}
ErrInteractionRequired = func(description string) *OAuthError {
return &OAuthError{
ErrorType: InteractionRequired,
Description: description,
}
}
ErrServerError = func(description string) *OAuthError {
return &OAuthError{
ErrorType: ServerError,
@ -45,7 +51,7 @@ type ErrAuthRequest interface {
GetState() string
}
func AuthRequestError(w http.ResponseWriter, r *http.Request, authReq ErrAuthRequest, err error, encoder *schema.Encoder) {
func AuthRequestError(w http.ResponseWriter, r *http.Request, authReq ErrAuthRequest, err error, encoder utils.Encoder) {
if authReq == nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
@ -56,7 +62,7 @@ func AuthRequestError(w http.ResponseWriter, r *http.Request, authReq ErrAuthReq
e.ErrorType = ServerError
e.Description = err.Error()
}
e.state = authReq.GetState()
e.State = authReq.GetState()
if authReq.GetRedirectURI() == "" || e.redirectDisabled {
http.Error(w, e.Description, http.StatusBadRequest)
return
@ -89,9 +95,9 @@ func RequestError(w http.ResponseWriter, r *http.Request, err error) {
type OAuthError struct {
ErrorType errorType `json:"error" schema:"error"`
Description string `json:"error_description" schema:"error_description"`
state string `json:"state" schema:"state"`
redirectDisabled bool
Description string `json:"error_description,omitempty" schema:"error_description,omitempty"`
State string `json:"state,omitempty" schema:"state,omitempty"`
redirectDisabled bool `json:"-" schema:"-"`
}
func (e *OAuthError) Error() string {

10
pkg/op/mock/authorizer.mock.go
View file

@ -7,8 +7,8 @@ package mock
import (
op "github.com/caos/oidc/pkg/op"
rp "github.com/caos/oidc/pkg/rp"
utils "github.com/caos/oidc/pkg/utils"
gomock "github.com/golang/mock/gomock"
schema "github.com/gorilla/schema"
reflect "reflect"
)
@ -50,10 +50,10 @@ func (mr *MockAuthorizerMockRecorder) Crypto() *gomock.Call {
}
// Decoder mocks base method
func (m *MockAuthorizer) Decoder() *schema.Decoder {
func (m *MockAuthorizer) Decoder() utils.Decoder {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "Decoder")
ret0, _ := ret[0].(*schema.Decoder)
ret0, _ := ret[0].(utils.Decoder)
return ret0
}
@ -64,10 +64,10 @@ func (mr *MockAuthorizerMockRecorder) Decoder() *gomock.Call {
}
// Encoder mocks base method
func (m *MockAuthorizer) Encoder() *schema.Encoder {
func (m *MockAuthorizer) Encoder() utils.Encoder {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "Encoder")
ret0, _ := ret[0].(*schema.Encoder)
ret0, _ := ret[0].(utils.Encoder)
return ret0
}

3
pkg/op/mock/authorizer.mock.impl.go
View file

@ -69,6 +69,9 @@ type Verifier struct{}
func (v *Verifier) Verify(ctx context.Context, accessToken, idToken string) (*oidc.IDTokenClaims, error) {
return nil, nil
}
func (v *Verifier) VerifyIDToken(ctx context.Context, idToken string) (*oidc.IDTokenClaims, error) {
return nil, nil
}
type Sig struct{}

6
pkg/op/mock/client.go
View file

@ -1,12 +1,12 @@
package mock
import (
"github.com/caos/oidc/pkg/oidc"
"testing"
gomock "github.com/golang/mock/gomock"
"github.com/golang/mock/gomock"
op "github.com/caos/oidc/pkg/op"
"github.com/caos/oidc/pkg/oidc"
"github.com/caos/oidc/pkg/op"
)
func NewClient(t *testing.T) op.Client {

8
pkg/op/session.go
View file

@ -6,11 +6,11 @@ import (
"github.com/caos/oidc/pkg/oidc"
"github.com/caos/oidc/pkg/rp"
"github.com/gorilla/schema"
"github.com/caos/oidc/pkg/utils"
)
type SessionEnder interface {
Decoder() *schema.Decoder
Decoder() utils.Decoder
Storage() Storage
IDTokenVerifier() rp.Verifier
DefaultLogoutRedirectURI() string
@ -39,7 +39,7 @@ func EndSession(w http.ResponseWriter, r *http.Request, ender SessionEnder) {
http.Redirect(w, r, session.RedirectURI, http.StatusFound)
}
func ParseEndSessionRequest(r *http.Request, decoder *schema.Decoder) (*oidc.EndSessionRequest, error) {
func ParseEndSessionRequest(r *http.Request, decoder utils.Decoder) (*oidc.EndSessionRequest, error) {
err := r.ParseForm()
if err != nil {
return nil, ErrInvalidRequest("error parsing form")
@ -57,7 +57,7 @@ func ValidateEndSessionRequest(ctx context.Context, req *oidc.EndSessionRequest,
if req.IdTokenHint == "" {
return session, nil
}
claims, err := ender.IDTokenVerifier().Verify(ctx, "", req.IdTokenHint)
claims, err := ender.IDTokenVerifier().VerifyIDToken(ctx, req.IdTokenHint)
if err != nil {
return nil, ErrInvalidRequest("id_token_hint invalid")
}

1
pkg/op/signer.go
View file

@ -8,6 +8,7 @@ import (
"gopkg.in/square/go-jose.v2"
"github.com/caos/logging"
"github.com/caos/oidc/pkg/oidc"
)

6
pkg/op/tokenrequest.go
View file

@ -5,8 +5,6 @@ import (
"errors"
"net/http"
"github.com/gorilla/schema"
"github.com/caos/oidc/pkg/oidc"
"github.com/caos/oidc/pkg/utils"
)
@ -14,7 +12,7 @@ import (
type Exchanger interface {
Issuer() string
Storage() Storage
Decoder() *schema.Decoder
Decoder() utils.Decoder
Signer() Signer
Crypto() Crypto
AuthMethodPostSupported() bool
@ -42,7 +40,7 @@ func CodeExchange(w http.ResponseWriter, r *http.Request, exchanger Exchanger) {
utils.MarshalJSON(w, resp)
}
func ParseAccessTokenRequest(r *http.Request, decoder *schema.Decoder) (*oidc.AccessTokenRequest, error) {
func ParseAccessTokenRequest(r *http.Request, decoder utils.Decoder) (*oidc.AccessTokenRequest, error) {
err := r.ParseForm()
if err != nil {
return nil, ErrInvalidRequest("error parsing form")

6
pkg/op/userinfo.go
View file

@ -5,14 +5,12 @@ import (
"net/http"
"strings"
"github.com/gorilla/schema"
"github.com/caos/oidc/pkg/oidc"
"github.com/caos/oidc/pkg/utils"
)
type UserinfoProvider interface {
Decoder() *schema.Decoder
Decoder() utils.Decoder
Crypto() Crypto
Storage() Storage
}
@ -37,7 +35,7 @@ func Userinfo(w http.ResponseWriter, r *http.Request, userinfoProvider UserinfoP
utils.MarshalJSON(w, info)
}
func getAccessToken(r *http.Request, decoder *schema.Decoder) (string, error) {
func getAccessToken(r *http.Request, decoder utils.Decoder) (string, error) {
authHeader := r.Header.Get("authorization")
if authHeader != "" {
parts := strings.Split(authHeader, "Bearer ")