From 7a202b5371bb6f8156b972a48cdede4ab86dc992 Mon Sep 17 00:00:00 2001 From: Martti Tamm Date: Fri, 2 Feb 2024 10:02:59 +0200 Subject: [PATCH 1/6] Issue #2: Bearer JWT (OIDC) authentication support --- config/config.go | 10 ++- config/default-config.yaml | 12 +++ go.mod | 5 +- go.sum | 30 ++++++++ server/auth.go | 98 +++++++++++------------- server/auth_oidc.go | 152 +++++++++++++++++++++++++++++++++++++ server/server.go | 3 +- tes/client.go | 21 +++-- 8 files changed, 270 insertions(+), 61 deletions(-) create mode 100644 server/auth_oidc.go diff --git a/config/config.go b/config/config.go index 18d77997..b42fecc2 100644 --- a/config/config.go +++ b/config/config.go @@ -56,6 +56,14 @@ type BasicCredential struct { Password string } +type OidcAuth struct { + ServiceConfigUrl string + ClientId string + ClientSecret string + RequireScope string + RequireAudience string +} + // RPCClient describes configuration for gRPC clients type RPCClient struct { BasicCredential @@ -77,6 +85,7 @@ type Server struct { HTTPPort string RPCPort string BasicAuth []BasicCredential + OidcAuth OidcAuth DisableHTTPCache bool } @@ -395,7 +404,6 @@ func (h HTSGETStorage) Valid() bool { return !h.Disabled } - // Kubernetes describes the configuration for the Kubernetes compute backend. type Kubernetes struct { // The executor used to execute tasks. Available executors: docker, kubernetes diff --git a/config/default-config.yaml b/config/default-config.yaml index 51de72b0..b155e454 100644 --- a/config/default-config.yaml +++ b/config/default-config.yaml @@ -37,6 +37,18 @@ Server: # - User: user2 # Password: foobar + # Require Bearer JWT authentication for the server APIs. + # Server won't launch when configuration URL cannot be loaded. + # OidcAuth: + # # URL of the OIDC service configuration: + # ServiceConfigUrl: "" + # ClientId: "" + # ClientSecret: "" + # # Optional: if specified, this scope value must be in the token: + # RequireScope: + # # Optional: if specified, this audience value must be in the token: + # RequireAudience: + # Include a "Cache-Control: no-store" HTTP header in Get/List responses # to prevent caching by intermediary services. DisableHTTPCache: true diff --git a/go.mod b/go.mod index c89a61c2..84d3fccc 100644 --- a/go.mod +++ b/go.mod @@ -38,6 +38,7 @@ require ( github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af // indirect github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/kr/pretty v0.3.1 + github.com/lestrrat-go/jwx/v2 v2.0.19 // indirect github.com/logrusorgru/aurora v0.0.0-20200102142835-e9ef32dff381 github.com/maruel/panicparse v1.3.0 // indirect github.com/mattn/go-runewidth v0.0.8 // indirect @@ -59,8 +60,8 @@ require ( github.com/smartystreets/goconvey v1.6.4 // indirect github.com/spf13/cobra v0.0.5 github.com/spf13/pflag v1.0.5 - github.com/stretchr/testify v1.8.3 - golang.org/x/crypto v0.14.0 + github.com/stretchr/testify v1.8.4 + golang.org/x/crypto v0.17.0 golang.org/x/net v0.17.0 golang.org/x/oauth2 v0.8.0 golang.org/x/time v0.3.0 diff --git a/go.sum b/go.sum index 3df6eb5d..d8616c9f 100644 --- a/go.sum +++ b/go.sum @@ -693,6 +693,9 @@ github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/decred/dcrd/crypto/blake256 v1.0.1/go.mod h1:2OfgNZ5wDpcsFmHmCK5gZTPcCXqlm2ArzUIkw9czNJo= +github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 h1:8UrgZ3GkP4i/CLijOJx79Yu+etlyjdBU4sfcs2WYQMs= +github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0/go.mod h1:v57UDF4pDQJcEfFUCRop3lJL149eHGSe9Jvczhzjo/0= github.com/dgraph-io/badger/v2 v2.0.1 h1:+D6dhIqC6jIeCclnxMHqk4HPuXgrRN5UfBsLR4dNQ3A= github.com/dgraph-io/badger/v2 v2.0.1/go.mod h1:YoRSIp1LmAJ7zH7tZwRvjNMUYLxB4wl3ebYkaIruZ04= github.com/dgraph-io/ristretto v0.0.0-20191025175511-c1f00be0418e h1:aeUNgwup7PnDOBAD1BOKAqzb/W/NksOj6r3dwKKuqfg= @@ -801,6 +804,8 @@ github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572/go.mod h1:9Pwr4 github.com/go-test/deep v1.0.5 h1:AKODKU3pDH1RzZzm6YZu77YWtEAq6uh1rLIAQlay2qc= github.com/go-test/deep v1.0.5/go.mod h1:QV8Hv/iy04NyLBxAdO9njL0iVPN1S4d/A3NVv1V36o8= github.com/goccy/go-json v0.9.11/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= +github.com/goccy/go-json v0.10.2 h1:CrxCmQqYDkv1z7lO7Wbh2HN93uovUHgrECaO5ZrCXAU= +github.com/goccy/go-json v0.10.2/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= @@ -995,6 +1000,19 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/lestrrat-go/blackmagic v1.0.2 h1:Cg2gVSc9h7sz9NOByczrbUvLopQmXrfFx//N+AkAr5k= +github.com/lestrrat-go/blackmagic v1.0.2/go.mod h1:UrEqBzIR2U6CnzVyUtfM6oZNMt/7O7Vohk2J0OGSAtU= +github.com/lestrrat-go/httpcc v1.0.1 h1:ydWCStUeJLkpYyjLDHihupbn2tYmZ7m22BGkcvZZrIE= +github.com/lestrrat-go/httpcc v1.0.1/go.mod h1:qiltp3Mt56+55GPVCbTdM9MlqhvzyuL6W/NMDA8vA5E= +github.com/lestrrat-go/httprc v1.0.4 h1:bAZymwoZQb+Oq8MEbyipag7iSq6YIga8Wj6GOiJGdI8= +github.com/lestrrat-go/httprc v1.0.4/go.mod h1:mwwz3JMTPBjHUkkDv/IGJ39aALInZLrhBp0X7KGUZlo= +github.com/lestrrat-go/iter v1.0.2 h1:gMXo1q4c2pHmC3dn8LzRhJfP1ceCbgSiT9lUydIzltI= +github.com/lestrrat-go/iter v1.0.2/go.mod h1:Momfcq3AnRlRjI5b5O8/G5/BvpzrhoFTZcn06fEOPt4= +github.com/lestrrat-go/jwx/v2 v2.0.19 h1:ekv1qEZE6BVct89QA+pRF6+4pCpfVrOnEJnTnT4RXoY= +github.com/lestrrat-go/jwx/v2 v2.0.19/go.mod h1:l3im3coce1lL2cDeAjqmaR+Awx+X8Ih+2k8BuHNJ4CU= +github.com/lestrrat-go/option v1.0.0/go.mod h1:5ZHFbivi4xwXxhxY9XHDe2FHo6/Z7WWmtT7T5nBBp3I= +github.com/lestrrat-go/option v1.0.1 h1:oAzP2fvZGQKWkvHa1/SAcFolBEca1oN+mQ7eooNBEYU= +github.com/lestrrat-go/option v1.0.1/go.mod h1:5ZHFbivi4xwXxhxY9XHDe2FHo6/Z7WWmtT7T5nBBp3I= github.com/logrusorgru/aurora v0.0.0-20200102142835-e9ef32dff381 h1:bqDmpDG49ZRnB5PcgP0RXtQvnMSgIF14M7CBd2shtXs= github.com/logrusorgru/aurora v0.0.0-20200102142835-e9ef32dff381/go.mod h1:7rIyQOR62GCctdiQpZ/zOJlFyk6y+94wXzv6RNZgaR4= github.com/lyft/protoc-gen-star v0.6.0/go.mod h1:TGAoBVkt8w7MPG72TrKIu85MIdXwDuzJYeZuUPFPNwA= @@ -1142,6 +1160,8 @@ github.com/russross/blackfriday v1.5.2 h1:HyvC0ARfnZBqnXwABFeSZHpKvJHJJfPz81GNue github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g= github.com/ruudk/golang-pdf417 v0.0.0-20181029194003-1af4ab5afa58/go.mod h1:6lfFZQK844Gfx8o5WFuvpxWRwnSoipWe/p622j1v06w= github.com/ruudk/golang-pdf417 v0.0.0-20201230142125-a7e3863a1245/go.mod h1:pQAZKsJ8yyVxGRWYNEm9oFB8ieLgKFnamEyDmSA0BRk= +github.com/segmentio/asm v1.2.0 h1:9BQrFxC+YOHJlTlHGkTrFWf59nbL3XnCoFLTwDCI7ys= +github.com/segmentio/asm v1.2.0/go.mod h1:BqMnlJP91P8d+4ibuonYZw9mfnzI9HfxselHZr5aAcs= github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0= github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= github.com/shirou/gopsutil v2.20.1+incompatible h1:oIq9Cq4i84Hk8uQAUOG3eNdI/29hBawGrD5YRl6JRDY= @@ -1194,6 +1214,8 @@ github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.3 h1:RP3t2pwF7cMEbC1dqtB6poj3niw/9gnV4Cjg5oW5gtY= github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c/go.mod h1:lB8K/P019DLNhemzwFU4jHLhdvlE6uDZjXFejJXr49I= github.com/xdg/stringprep v1.0.0/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0HrGL1Y= @@ -1237,6 +1259,8 @@ golang.org/x/crypto v0.0.0-20211108221036-ceb1ce70b4fa/go.mod h1:GvvjBRRGRdwPK5y golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw= golang.org/x/crypto v0.14.0 h1:wBqGXzWJW6m1XrIKlAH0Hs1JJ7+9KBwnIO8v66Q9cHc= golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4= +golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k= +golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -1509,6 +1533,8 @@ golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= +golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= @@ -1521,6 +1547,8 @@ golang.org/x/term v0.7.0/go.mod h1:P32HKFT3hSsZrRxla30E9HqToFYAQPCMs/zFMBUFqPY= golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= golang.org/x/term v0.13.0 h1:bb+I9cTfFazGW51MZqBVmZy7+JEJMouUHTUSKVQLBek= golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U= +golang.org/x/term v0.15.0 h1:y/Oo/a/q3IXu26lQgl04j/gjuBDOBlx7X6Om1j2CPW4= +golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -1539,6 +1567,8 @@ golang.org/x/text v0.8.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k= golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= +golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= +golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/time v0.0.0-20170424234030-8be79e1e0910/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= diff --git a/server/auth.go b/server/auth.go index 5d2d9d92..230da179 100644 --- a/server/auth.go +++ b/server/auth.go @@ -9,11 +9,21 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" ) -// Return a new interceptor function that authorizes RPCs -// using a password stored in the config. -func newAuthInterceptor(creds []config.BasicCredential) grpc.UnaryServerInterceptor { +var ( + errMissingMetadata = status.Errorf(codes.InvalidArgument, "Missing metadata in the context") + errTokenRequired = status.Errorf(codes.Unauthenticated, "Bearer/Basic authorization token missing") + errInvalidBasicToken = status.Errorf(codes.Unauthenticated, "Invalid Basic authorization token") + errInvalidBearerToken = status.Errorf(codes.Unauthenticated, "Invalid Bearer authorization token") +) + +// Return a new interceptor function that authorizes RPCs. +func newAuthInterceptor(creds []config.BasicCredential, oidc config.OidcAuth) grpc.UnaryServerInterceptor { + basicCreds := initBasicCredsMap(creds) + oidcConfig := initOidcConfig(oidc) + requireAuth := len(basicCreds) > 0 || oidcConfig != nil // Return a function that is the interceptor. return func( @@ -21,65 +31,49 @@ func newAuthInterceptor(creds []config.BasicCredential) grpc.UnaryServerIntercep req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { - var authorized bool - var err error - for _, cred := range creds { - err = authorize(ctx, cred.User, cred.Password) - if err == nil { - authorized = true - } - } - if len(creds) == 0 { - authorized = true - } - if !authorized { - return nil, err + + if !requireAuth { + return handler(ctx, req) } - return handler(ctx, req) - } -} -// Check the context's metadata for the configured server/API password. -func authorize(ctx context.Context, user, password string) error { - if md, ok := metadata.FromIncomingContext(ctx); ok { - if len(md["authorization"]) > 0 { - raw := md["authorization"][0] - requser, reqpass, ok := parseBasicAuth(raw) - if ok { - if requser == user && reqpass == password { - return nil - } - return grpc.Errorf(codes.PermissionDenied, "") - } + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return nil, errMissingMetadata } - } - return grpc.Errorf(codes.Unauthenticated, "") -} + values := md["authorization"] + if len(values) < 1 { + return nil, errTokenRequired + } -// parseBasicAuth parses an HTTP Basic Authentication string. -// "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==" returns ("Aladdin", "open sesame", true). -// -// Taken from Go core: https://golang.org/src/net/http/request.go?s=27379:27445#L828 -func parseBasicAuth(auth string) (username, password string, ok bool) { - const prefix = "Basic " + authorized := false + auth_err := errTokenRequired - if !strings.HasPrefix(auth, prefix) { - return - } + if strings.HasPrefix(values[0], "Basic ") { + auth_err = errInvalidBasicToken + authorized = basicCreds[values[0]] + } else if oidcConfig != nil && strings.HasPrefix(values[0], "Bearer ") { + auth_err = errInvalidBearerToken + jwtString := strings.TrimPrefix(values[0], "Bearer ") + jwt := oidcConfig.ParseJwt(jwtString) + authorized = jwt != nil + } - c, err := base64.StdEncoding.DecodeString(auth[len(prefix):]) + if !authorized { + return nil, auth_err + } - if err != nil { - return + return handler(ctx, req) } +} - cs := string(c) - s := strings.IndexByte(cs, ':') - - if s < 0 { - return +func initBasicCredsMap(creds []config.BasicCredential) map[string]bool { + basicCreds := make(map[string]bool) + for _, cred := range creds { + credBytes := []byte(cred.User + ":" + cred.Password) + fullValue := "Basic " + base64.StdEncoding.EncodeToString(credBytes) + basicCreds[fullValue] = true } - return cs[:s], cs[s+1:], true + return basicCreds } diff --git a/server/auth_oidc.go b/server/auth_oidc.go new file mode 100644 index 00000000..586c5d3a --- /dev/null +++ b/server/auth_oidc.go @@ -0,0 +1,152 @@ +package server + +import ( + "encoding/json" + "io" + "log" + "net/http" + "net/url" + "strings" + "time" + + "github.com/lestrrat-go/jwx/v2/jwk" + "github.com/lestrrat-go/jwx/v2/jwt" + "github.com/ohsu-comp-bio/funnel/config" + "golang.org/x/net/context" +) + +// JSON structure of the OIDC configuration (only some fields) +type OidcRemoteConfig struct { + Issuer string `json:"issuer"` + UserinfoEndpoint string `json:"userinfo_endpoint"` + JwksURI string `json:"jwks_uri"` +} + +// OIDC configuration structure used for validating input from request. +type OidcConfig struct { + local config.OidcAuth + remote OidcRemoteConfig + jwks jwk.Cache +} + +func initOidcConfig(config config.OidcAuth) *OidcConfig { + if config.ServiceConfigUrl == "" || + config.ClientId == "" || + config.ClientSecret == "" { + return nil + } + + result := OidcConfig{local: config} + result.initConfig() + return &result +} + +func (c *OidcConfig) initConfig() { + c.remote = OidcRemoteConfig{} + parsedUrl := validateUrl(c.local.ServiceConfigUrl) + err := json.Unmarshal(fetchJson(parsedUrl), &c.remote) + if err != nil { + log.Fatalf("Failed to parse the configuration (JSON) of the OIDC "+ + "service: %s", err) + } + + c.initJwks() +} + +func (c *OidcConfig) initJwks() { + jwksUrl := c.remote.JwksURI + ctx := context.Background() + + // Define JWKS cache: + c.jwks = *jwk.NewCache(ctx) + c.jwks.Register(jwksUrl, jwk.WithMinRefreshInterval(1*time.Hour)) + + // Init JWKS cache: + ctx2, _ := context.WithTimeout(ctx, 10*time.Second) + _, err := c.jwks.Refresh(ctx2, jwksUrl) + + if err != nil { + log.Fatalf("Failed to fetch JWKS (%s) of the OIDC service (%s).", + jwksUrl, c.local.ServiceConfigUrl, err) + } +} + +func (c *OidcConfig) ParseJwt(jwtString string) *jwt.Token { + keySet, err := c.jwks.Get(context.Background(), c.remote.JwksURI) + if err != nil { + log.Println("Failed to retrieve JWKS key-set.", err) + return nil + } + + token, err := jwt.ParseString( + jwtString, + jwt.WithVerify(true), + jwt.WithKeySet(keySet), + jwt.WithIssuer(c.remote.Issuer), + ) + + // If audience is required, it must be in the token. + if c.local.RequireAudience != "" { + found := false + for _, value := range token.Audience() { + if value == c.local.RequireAudience { + found = true + break + } + } + if !found { + return nil + } + } + + // If scope is required, it must be in the token. + if c.local.RequireScope != "" { + value, found := token.Get("scope") + if found { + found = false + for _, value := range strings.Split(value.(string), " ") { + if value == c.local.RequireScope { + found = true + break + } + } + } + if !found { + return nil + } + } + + return &token +} + +func validateUrl(providedUrl string) *url.URL { + parsedUrl, err := url.ParseRequestURI(providedUrl) + if err != nil { + log.Fatalf("OIDC configuration URL (%s) could not be parsed.", parsedUrl, err) + } else if parsedUrl.Scheme == "" || parsedUrl.Host == "" { + log.Fatalf("OIDC configuration URL (%s) is not absolute.", parsedUrl) + } + return parsedUrl +} + +func fetchJson(url *url.URL) []byte { + res, err := http.Get(url.String()) + + if err != nil { + log.Fatal("OIDC service configuration could not be loaded", err) + } else if res.StatusCode != 200 { + log.Fatalf("OIDC service configuration could not be loaded (HTTP "+ + " response status: %d)", res.StatusCode) + } else if res.Body == nil { + log.Fatal("OIDC service configuration could not be loaded (empty " + + "response)") + } + + defer res.Body.Close() + body, err := io.ReadAll(res.Body) + if err != nil { + log.Fatal("Failed to read the body of the OIDC configuration response", err) + } + + return body +} diff --git a/server/server.go b/server/server.go index df492f34..3640755d 100644 --- a/server/server.go +++ b/server/server.go @@ -26,6 +26,7 @@ type Server struct { RPCAddress string HTTPPort string BasicAuth []config.BasicCredential + OidcAuth config.OidcAuth Tasks tes.TaskServiceServer Events events.EventServiceServer Nodes scheduler.SchedulerServiceServer @@ -68,7 +69,7 @@ func (s *Server) Serve(pctx context.Context) error { grpc.UnaryInterceptor( grpc_middleware.ChainUnaryServer( // API auth check. - newAuthInterceptor(s.BasicAuth), + newAuthInterceptor(s.BasicAuth, s.OidcAuth), newDebugInterceptor(s.Log), ), ), diff --git a/tes/client.go b/tes/client.go index 48533e53..501d8761 100644 --- a/tes/client.go +++ b/tes/client.go @@ -22,6 +22,7 @@ import ( func NewClient(address string) (*Client, error) { user := os.Getenv("FUNNEL_SERVER_USER") password := os.Getenv("FUNNEL_SERVER_PASSWORD") + bearer := os.Getenv("FUNNEL_SERVER_BEARER") re := regexp.MustCompile("^(.+://)?(.[^/]+)(.+)?$") endpoint := re.ReplaceAllString(address, "$1$2") @@ -43,6 +44,7 @@ func NewClient(address string) (*Client, error) { Marshaler: &Marshaler, User: user, Password: password, + Bearer: bearer, }, nil } @@ -53,6 +55,15 @@ type Client struct { Marshaler *jsonpb.Marshaler User string Password string + Bearer string +} + +func (c *Client) setAuth(hreq *http.Request) { + if c.User != "" && c.Password != "" { + hreq.SetBasicAuth(c.User, c.Password) + } else if c.Bearer != "" { + hreq.Header.Set("Authorization", "Bearer "+c.Bearer) + } } // GetTask returns the raw bytes from GET /v1/tasks/{id} @@ -61,7 +72,7 @@ func (c *Client) GetTask(ctx context.Context, req *GetTaskRequest) (*Task, error u := c.address + "/v1/tasks/" + req.Id + "?view=" + req.View.String() hreq, _ := http.NewRequest("GET", u, nil) hreq.WithContext(ctx) - hreq.SetBasicAuth(c.User, c.Password) + c.setAuth(hreq) body, err := util.CheckHTTPResponse(c.client.Do(hreq)) if err != nil { return nil, err @@ -95,7 +106,7 @@ func (c *Client) ListTasks(ctx context.Context, req *ListTasksRequest) (*ListTas u := c.address + "/v1/tasks?" + v.Encode() hreq, _ := http.NewRequest("GET", u, nil) hreq.WithContext(ctx) - hreq.SetBasicAuth(c.User, c.Password) + c.setAuth(hreq) body, err := util.CheckHTTPResponse(c.client.Do(hreq)) if err != nil { return nil, err @@ -127,7 +138,7 @@ func (c *Client) CreateTask(ctx context.Context, task *Task) (*CreateTaskRespons hreq, _ := http.NewRequest("POST", u, &b) hreq.WithContext(ctx) hreq.Header.Add("Content-Type", "application/json") - hreq.SetBasicAuth(c.User, c.Password) + c.setAuth(hreq) body, err := util.CheckHTTPResponse(c.client.Do(hreq)) if err != nil { return nil, err @@ -148,7 +159,7 @@ func (c *Client) CancelTask(ctx context.Context, req *CancelTaskRequest) (*Cance hreq, _ := http.NewRequest("POST", u, nil) hreq.WithContext(ctx) hreq.Header.Add("Content-Type", "application/json") - hreq.SetBasicAuth(c.User, c.Password) + c.setAuth(hreq) body, err := util.CheckHTTPResponse(c.client.Do(hreq)) if err != nil { return nil, err @@ -168,7 +179,7 @@ func (c *Client) GetServiceInfo(ctx context.Context, req *ServiceInfoRequest) (* u := c.address + "/v1/tasks/service-info" hreq, _ := http.NewRequest("GET", u, nil) hreq.WithContext(ctx) - hreq.SetBasicAuth(c.User, c.Password) + c.setAuth(hreq) body, err := util.CheckHTTPResponse(c.client.Do(hreq)) if err != nil { return nil, err From 65ab8aea891060a8eb299d334a150f7da753f777 Mon Sep 17 00:00:00 2001 From: Martti Tamm Date: Sat, 3 Feb 2024 11:16:40 +0200 Subject: [PATCH 2/6] Issue #2: Fixes and a documentation page about the new config --- cmd/server/run.go | 1 + cmd/util/config.go | 28 +++++++++++++++ config/config.go | 2 -- config/default-config.yaml | 2 -- server/auth.go | 2 +- server/auth_oidc.go | 12 +++++-- website/content/docs/security/oauth2.md | 46 +++++++++++++++++++++++++ 7 files changed, 85 insertions(+), 8 deletions(-) create mode 100644 website/content/docs/security/oauth2.md diff --git a/cmd/server/run.go b/cmd/server/run.go index 0ffc053d..869edfa4 100644 --- a/cmd/server/run.go +++ b/cmd/server/run.go @@ -263,6 +263,7 @@ func NewServer(ctx context.Context, conf config.Config, log *logger.Logger) (*Se RPCAddress: ":" + conf.Server.RPCPort, HTTPPort: conf.Server.HTTPPort, BasicAuth: conf.Server.BasicAuth, + OidcAuth: conf.Server.OidcAuth, DisableHTTPCache: conf.Server.DisableHTTPCache, Log: log, Tasks: &server.TaskService{ diff --git a/cmd/util/config.go b/cmd/util/config.go index 70b280aa..8a5b09ad 100644 --- a/cmd/util/config.go +++ b/cmd/util/config.go @@ -2,6 +2,8 @@ package util import ( "io/ioutil" + "log" + "math/rand" "os" "path/filepath" @@ -34,6 +36,22 @@ func MergeConfigFileWithFlags(file string, flagConf config.Config) (config.Confi } } + // Make sure that User/Password are defined for conf.RPCClient: + // 1) when conf.Server.BasicAuth has credentials + // 2) when conf.Server.OidcAuth is enabled (clients still need to provide Basic credentials) + if conf.RPCClient.User == "" && conf.RPCClient.Password == "" { + if len(conf.Server.BasicAuth) > 0 { + log.Fatal("RPCClient User and Password are undefined while Server.BasicAuth is enabled.") + } else if conf.Server.OidcAuth.ServiceConfigUrl != "" { + // Generating random user/password credentials for RPC: + conf.RPCClient.User = randomCredential() + conf.RPCClient.Password = randomCredential() + conf.Server.BasicAuth = append(conf.Server.BasicAuth, config.BasicCredential{ + User: conf.RPCClient.User, + Password: conf.RPCClient.Password, + }) + } + } return conf, nil } @@ -58,3 +76,13 @@ func TempConfigFile(c config.Config, name string) (path string, cleanup func()) } return p, cleanup } + +// RandomString generates a random string of length 20. +func randomCredential() string { + var letterRunes = []rune("abcdefghijklmnopqrstuvwxyz0123456789") + b := make([]rune, 20) + for i := range b { + b[i] = letterRunes[rand.Intn(len(letterRunes))] + } + return string(b) +} diff --git a/config/config.go b/config/config.go index b42fecc2..f5bfd1de 100644 --- a/config/config.go +++ b/config/config.go @@ -58,8 +58,6 @@ type BasicCredential struct { type OidcAuth struct { ServiceConfigUrl string - ClientId string - ClientSecret string RequireScope string RequireAudience string } diff --git a/config/default-config.yaml b/config/default-config.yaml index b155e454..b2608518 100644 --- a/config/default-config.yaml +++ b/config/default-config.yaml @@ -42,8 +42,6 @@ Server: # OidcAuth: # # URL of the OIDC service configuration: # ServiceConfigUrl: "" - # ClientId: "" - # ClientSecret: "" # # Optional: if specified, this scope value must be in the token: # RequireScope: # # Optional: if specified, this audience value must be in the token: diff --git a/server/auth.go b/server/auth.go index 230da179..098adeb4 100644 --- a/server/auth.go +++ b/server/auth.go @@ -14,7 +14,7 @@ import ( var ( errMissingMetadata = status.Errorf(codes.InvalidArgument, "Missing metadata in the context") - errTokenRequired = status.Errorf(codes.Unauthenticated, "Bearer/Basic authorization token missing") + errTokenRequired = status.Errorf(codes.Unauthenticated, "Basic/Bearer authorization token missing") errInvalidBasicToken = status.Errorf(codes.Unauthenticated, "Invalid Basic authorization token") errInvalidBearerToken = status.Errorf(codes.Unauthenticated, "Invalid Bearer authorization token") ) diff --git a/server/auth_oidc.go b/server/auth_oidc.go index 586c5d3a..4e4f3c9f 100644 --- a/server/auth_oidc.go +++ b/server/auth_oidc.go @@ -2,6 +2,7 @@ package server import ( "encoding/json" + "fmt" "io" "log" "net/http" @@ -30,9 +31,7 @@ type OidcConfig struct { } func initOidcConfig(config config.OidcAuth) *OidcConfig { - if config.ServiceConfigUrl == "" || - config.ClientId == "" || - config.ClientSecret == "" { + if config.ServiceConfigUrl == "" { return nil } @@ -85,6 +84,11 @@ func (c *OidcConfig) ParseJwt(jwtString string) *jwt.Token { jwt.WithIssuer(c.remote.Issuer), ) + if err != nil { + fmt.Println("Token is not valid.", err) + return nil + } + // If audience is required, it must be in the token. if c.local.RequireAudience != "" { found := false @@ -95,6 +99,7 @@ func (c *OidcConfig) ParseJwt(jwtString string) *jwt.Token { } } if !found { + fmt.Printf("Audience [%s] not found in %v.", c.local.RequireAudience, token.Audience()) return nil } } @@ -112,6 +117,7 @@ func (c *OidcConfig) ParseJwt(jwtString string) *jwt.Token { } } if !found { + fmt.Printf("Scope [%s] not found in [%s]", c.local.RequireScope, value) return nil } } diff --git a/website/content/docs/security/oauth2.md b/website/content/docs/security/oauth2.md new file mode 100644 index 00000000..a022454f --- /dev/null +++ b/website/content/docs/security/oauth2.md @@ -0,0 +1,46 @@ +--- +title: OAuth2 +menu: + main: + parent: Security + weight: 10 +--- +# OAuth2 + +By default, a Funnel server allows open access to its API endpoints, but in +addition to Basic authentication it can also be configured to require a valid +JWT in the request. + +Funnel itself does not redirect users to perform the login. +It just validates that the presented token is issued by a trusted service +(specified in the configuration file) and the token has not expired. + +Optionally, Funnel can also validate the scope and audience claims to contain +specific values. + +To enable JWT authentication, specify `OidcAuth` section in your config file: + +```yaml +Server: + OidcAuth: + # URL of the OIDC service configuration: + ServiceConfigUrl: "https://my.oidc.service/.well-known/openid-configuration" + # Optional: if specified, this scope value must be in the token: + RequireScope: funnel-id + # Optional: if specified, this audience value must be in the token: + RequireAudience: tes-api +``` + +Make sure to properly protect the configuration file so that it's not readable +by everyone: + +```bash +$ chmod 600 funnel.config.yml +``` + +To use the token, set the `FUNNEL_SERVER_BEARER` environment variable: + +```bash +$ export FUNNEL_SERVER_BEARER=eyJraWQiOiJyc2ExIiwi... +$ funnel task list +``` From 24008434d4a5fde26a345a57bbc1d0e00827385d Mon Sep 17 00:00:00 2001 From: Martti Tamm Date: Sat, 3 Feb 2024 13:31:26 +0200 Subject: [PATCH 3/6] Changed problem logging --- cmd/util/config.go | 6 ++++-- server/auth_oidc.go | 50 +++++++++++++++++++++++++++++---------------- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/cmd/util/config.go b/cmd/util/config.go index 8a5b09ad..3a8c0b48 100644 --- a/cmd/util/config.go +++ b/cmd/util/config.go @@ -1,8 +1,8 @@ package util import ( + "fmt" "io/ioutil" - "log" "math/rand" "os" "path/filepath" @@ -41,7 +41,9 @@ func MergeConfigFileWithFlags(file string, flagConf config.Config) (config.Confi // 2) when conf.Server.OidcAuth is enabled (clients still need to provide Basic credentials) if conf.RPCClient.User == "" && conf.RPCClient.Password == "" { if len(conf.Server.BasicAuth) > 0 { - log.Fatal("RPCClient User and Password are undefined while Server.BasicAuth is enabled.") + fmt.Println("Configuration problem: RPCClient User and Password " + + "are undefined while Server.BasicAuth is enforeced.") + os.Exit(1) } else if conf.Server.OidcAuth.ServiceConfigUrl != "" { // Generating random user/password credentials for RPC: conf.RPCClient.User = randomCredential() diff --git a/server/auth_oidc.go b/server/auth_oidc.go index 4e4f3c9f..014bb4ad 100644 --- a/server/auth_oidc.go +++ b/server/auth_oidc.go @@ -4,9 +4,9 @@ import ( "encoding/json" "fmt" "io" - "log" "net/http" "net/url" + "os" "strings" "time" @@ -45,8 +45,9 @@ func (c *OidcConfig) initConfig() { parsedUrl := validateUrl(c.local.ServiceConfigUrl) err := json.Unmarshal(fetchJson(parsedUrl), &c.remote) if err != nil { - log.Fatalf("Failed to parse the configuration (JSON) of the OIDC "+ - "service: %s", err) + fmt.Printf("[ERROR] Failed to parse the configuration (JSON) of the "+ + "OIDC service: %s\n", err) + os.Exit(1) } c.initJwks() @@ -58,22 +59,23 @@ func (c *OidcConfig) initJwks() { // Define JWKS cache: c.jwks = *jwk.NewCache(ctx) - c.jwks.Register(jwksUrl, jwk.WithMinRefreshInterval(1*time.Hour)) + c.jwks.Register(jwksUrl, jwk.WithMinRefreshInterval(15*time.Minute)) // Init JWKS cache: ctx2, _ := context.WithTimeout(ctx, 10*time.Second) _, err := c.jwks.Refresh(ctx2, jwksUrl) if err != nil { - log.Fatalf("Failed to fetch JWKS (%s) of the OIDC service (%s).", - jwksUrl, c.local.ServiceConfigUrl, err) + fmt.Printf("[ERROR] Failed to fetch JWKS (%s) of the OIDC service "+ + "(%s): %s\n", jwksUrl, c.local.ServiceConfigUrl, err) + os.Exit(1) } } func (c *OidcConfig) ParseJwt(jwtString string) *jwt.Token { keySet, err := c.jwks.Get(context.Background(), c.remote.JwksURI) if err != nil { - log.Println("Failed to retrieve JWKS key-set.", err) + fmt.Printf("[WARN] Failed to retrieve JWKS key-set: %s", err) return nil } @@ -85,7 +87,7 @@ func (c *OidcConfig) ParseJwt(jwtString string) *jwt.Token { ) if err != nil { - fmt.Println("Token is not valid.", err) + fmt.Printf("[WARN] Provided JWT is not valid: %s.\n", err) return nil } @@ -99,7 +101,8 @@ func (c *OidcConfig) ParseJwt(jwtString string) *jwt.Token { } } if !found { - fmt.Printf("Audience [%s] not found in %v.", c.local.RequireAudience, token.Audience()) + fmt.Printf("[WARN] Audience [%s] not found in %v.", + c.local.RequireAudience, token.Audience()) return nil } } @@ -117,7 +120,8 @@ func (c *OidcConfig) ParseJwt(jwtString string) *jwt.Token { } } if !found { - fmt.Printf("Scope [%s] not found in [%s]", c.local.RequireScope, value) + fmt.Printf("[WARN] Scope [%s] not found in [%s]", + c.local.RequireScope, value) return nil } } @@ -128,9 +132,13 @@ func (c *OidcConfig) ParseJwt(jwtString string) *jwt.Token { func validateUrl(providedUrl string) *url.URL { parsedUrl, err := url.ParseRequestURI(providedUrl) if err != nil { - log.Fatalf("OIDC configuration URL (%s) could not be parsed.", parsedUrl, err) + fmt.Printf("[ERROR] OIDC configuration URL (%s) could not be "+ + "parsed: %s\n", parsedUrl, err) + os.Exit(1) } else if parsedUrl.Scheme == "" || parsedUrl.Host == "" { - log.Fatalf("OIDC configuration URL (%s) is not absolute.", parsedUrl) + fmt.Printf("[ERROR] OIDC configuration URL (%s) is not absolute.", + parsedUrl) + os.Exit(1) } return parsedUrl } @@ -139,19 +147,25 @@ func fetchJson(url *url.URL) []byte { res, err := http.Get(url.String()) if err != nil { - log.Fatal("OIDC service configuration could not be loaded", err) + fmt.Printf("[ERROR] OIDC service configuration (%s) could not be "+ + "loaded: %s.\n", url.String(), err) + os.Exit(1) } else if res.StatusCode != 200 { - log.Fatalf("OIDC service configuration could not be loaded (HTTP "+ - " response status: %d)", res.StatusCode) + fmt.Printf("[ERROR] OIDC service configuration (%s) could not be "+ + "loaded (HTTP response status: %d).", url.String(), res.StatusCode) + os.Exit(1) } else if res.Body == nil { - log.Fatal("OIDC service configuration could not be loaded (empty " + - "response)") + fmt.Printf("[ERROR] OIDC service configuration (%s) could not be "+ + "loaded (empty response).\n", url.String()) + os.Exit(1) } defer res.Body.Close() body, err := io.ReadAll(res.Body) if err != nil { - log.Fatal("Failed to read the body of the OIDC configuration response", err) + fmt.Printf("[ERROR] Failed to read the body of the OIDC "+ + "configuration (%s) response: %s\n", url.String(), err) + os.Exit(1) } return body From 690caa5bd5df8b07dcbf8ced1c497752f7a18645 Mon Sep 17 00:00:00 2001 From: Martti Tamm Date: Tue, 6 Feb 2024 14:10:54 +0200 Subject: [PATCH 4/6] Add token introspection call to check if token is active --- cmd/util/config.go | 2 +- config/config.go | 2 + config/default-config.yaml | 4 + server/auth_oidc.go | 123 ++++++++++++++++++++++-- website/content/docs/security/oauth2.md | 12 ++- 5 files changed, 131 insertions(+), 12 deletions(-) diff --git a/cmd/util/config.go b/cmd/util/config.go index 3a8c0b48..c017e3a7 100644 --- a/cmd/util/config.go +++ b/cmd/util/config.go @@ -42,7 +42,7 @@ func MergeConfigFileWithFlags(file string, flagConf config.Config) (config.Confi if conf.RPCClient.User == "" && conf.RPCClient.Password == "" { if len(conf.Server.BasicAuth) > 0 { fmt.Println("Configuration problem: RPCClient User and Password " + - "are undefined while Server.BasicAuth is enforeced.") + "are undefined while Server.BasicAuth is enforced.") os.Exit(1) } else if conf.Server.OidcAuth.ServiceConfigUrl != "" { // Generating random user/password credentials for RPC: diff --git a/config/config.go b/config/config.go index f5bfd1de..b42fecc2 100644 --- a/config/config.go +++ b/config/config.go @@ -58,6 +58,8 @@ type BasicCredential struct { type OidcAuth struct { ServiceConfigUrl string + ClientId string + ClientSecret string RequireScope string RequireAudience string } diff --git a/config/default-config.yaml b/config/default-config.yaml index b2608518..82ef1ee3 100644 --- a/config/default-config.yaml +++ b/config/default-config.yaml @@ -42,6 +42,10 @@ Server: # OidcAuth: # # URL of the OIDC service configuration: # ServiceConfigUrl: "" + # # Client ID and secret are sent with the token introspection request + # # (Basic authentication): + # ClientId: + # ClientSecret: # # Optional: if specified, this scope value must be in the token: # RequireScope: # # Optional: if specified, this audience value must be in the token: diff --git a/server/auth_oidc.go b/server/auth_oidc.go index 014bb4ad..ab07dd66 100644 --- a/server/auth_oidc.go +++ b/server/auth_oidc.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/http" + "net/http/httputil" "net/url" "os" "strings" @@ -18,9 +19,14 @@ import ( // JSON structure of the OIDC configuration (only some fields) type OidcRemoteConfig struct { - Issuer string `json:"issuer"` - UserinfoEndpoint string `json:"userinfo_endpoint"` - JwksURI string `json:"jwks_uri"` + Issuer string `json:"issuer"` + JwksURI string `json:"jwks_uri"` + IntrospectionEndpoint string `json:"introspection_endpoint"` +} + +// JSON structure of the OIDC token introspection response (only some fields) +type IntrospectionResponse struct { + Active bool `json:"active"` } // OIDC configuration structure used for validating input from request. @@ -91,10 +97,18 @@ func (c *OidcConfig) ParseJwt(jwtString string) *jwt.Token { return nil } + if !c.isJwtValid(&token) || !c.isJwtActive(jwtString) { + return nil + } + + return &token +} + +func (c *OidcConfig) isJwtValid(token *jwt.Token) bool { // If audience is required, it must be in the token. if c.local.RequireAudience != "" { found := false - for _, value := range token.Audience() { + for _, value := range (*token).Audience() { if value == c.local.RequireAudience { found = true break @@ -102,14 +116,14 @@ func (c *OidcConfig) ParseJwt(jwtString string) *jwt.Token { } if !found { fmt.Printf("[WARN] Audience [%s] not found in %v.", - c.local.RequireAudience, token.Audience()) - return nil + c.local.RequireAudience, (*token).Audience()) + return false } } // If scope is required, it must be in the token. if c.local.RequireScope != "" { - value, found := token.Get("scope") + value, found := (*token).Get("scope") if found { found = false for _, value := range strings.Split(value.(string), " ") { @@ -122,11 +136,100 @@ func (c *OidcConfig) ParseJwt(jwtString string) *jwt.Token { if !found { fmt.Printf("[WARN] Scope [%s] not found in [%s]", c.local.RequireScope, value) - return nil + return false } } - return &token + return true +} + +func (c *OidcConfig) isJwtActive(token string) bool { + if c.remote.IntrospectionEndpoint == "" { + fmt.Println("[WARN] JWT introspection endpoint was not defined in the OIDC " + + "(remote) configuration; therefore assuming that the token is active.") + return true + } + + client := &http.Client{} + params := url.Values{"token": {token}}.Encode() + attemptsCount := 3 + + for attemptsCount > 0 { + request, err := http.NewRequest( + http.MethodPost, + c.remote.IntrospectionEndpoint, + strings.NewReader(params)) + + request.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + if c.local.ClientId != "" && c.local.ClientSecret != "" { + request.SetBasicAuth(c.local.ClientId, c.local.ClientSecret) + } else { + fmt.Println("[WARN] Requesting token introspection without " + + "client credentials (unspecified in the config)") + } + + response, err := client.Do(request) + + if err != nil { + fmt.Printf("[ERROR] Failed to call OIDC introspection endpoint "+ + "(POST %s): %s\n", c.remote.IntrospectionEndpoint, err) + if attemptsCount > 1 { + fmt.Println("Trying to call OIDC introspection endpoint again after a second...") + time.Sleep(1 * time.Second) + } else { + fmt.Println("[ERROR] Too many failed attempts for JWT " + + "introspection. Giving up.") + } + attemptsCount-- + continue + } + + defer response.Body.Close() + + if response.StatusCode != http.StatusOK { + byteDump, err := httputil.DumpResponse(response, true) + if err != nil { + fmt.Printf("Failed to dump response: %s\n", err) + } else { + fmt.Print(string(byteDump)) + } + fmt.Printf("[WARN] JWT introspection call gave non-200 HTTP status %d "+ + "(thus JWT not active)\n", response.StatusCode) + return false + } + + body, err := io.ReadAll(response.Body) + + if err != nil { + fmt.Printf("[WARN] Failed to read JWT introspection response "+ + "body with HTTP status %d (thus JWT not active): %s\n", + response.StatusCode, err) + return false + } + + if !strings.HasPrefix(response.Header.Get("Content-Type"), "application/json") { + fmt.Printf("[WARN] JWT introspection endpoint returned non-JSON "+ + "[content-type=%s] HTTP 200 response (thus JWT not active): %s\n", + response.Header.Get("Content-Type"), body) + return false + } + + if len(body) == 0 { + fmt.Println("[WARN] JWT introspection endpoint returned empty " + + "HTTP 200 response (thus JWT not active)") + return false + } + + var result IntrospectionResponse + if err := json.Unmarshal(body, &result); err != nil { + fmt.Printf("Cannot unmarshal JSON from the JWT introspection endpoint: %s", err) + } + + return result.Active + } + + return false } func validateUrl(providedUrl string) *url.URL { @@ -150,7 +253,7 @@ func fetchJson(url *url.URL) []byte { fmt.Printf("[ERROR] OIDC service configuration (%s) could not be "+ "loaded: %s.\n", url.String(), err) os.Exit(1) - } else if res.StatusCode != 200 { + } else if res.StatusCode != http.StatusOK { fmt.Printf("[ERROR] OIDC service configuration (%s) could not be "+ "loaded (HTTP response status: %d).", url.String(), res.StatusCode) os.Exit(1) diff --git a/website/content/docs/security/oauth2.md b/website/content/docs/security/oauth2.md index a022454f..64310e9e 100644 --- a/website/content/docs/security/oauth2.md +++ b/website/content/docs/security/oauth2.md @@ -13,7 +13,10 @@ JWT in the request. Funnel itself does not redirect users to perform the login. It just validates that the presented token is issued by a trusted service -(specified in the configuration file) and the token has not expired. +(specified in the YAML configuration file) and the token has not expired. +In addition, if the OIDC provides a token introspection endpoint (in its +configuration JSON), Funnel server also calls that endpoint to make sure the +token is still active (i.e., no token invalidation before expiring). Optionally, Funnel can also validate the scope and audience claims to contain specific values. @@ -25,8 +28,15 @@ Server: OidcAuth: # URL of the OIDC service configuration: ServiceConfigUrl: "https://my.oidc.service/.well-known/openid-configuration" + + # Client ID and secret are sent with the token introspection request + # (Basic authentication): + ClientId: your-client-id + ClientSecret: your-client-secret + # Optional: if specified, this scope value must be in the token: RequireScope: funnel-id + # Optional: if specified, this audience value must be in the token: RequireAudience: tes-api ``` From bde1d83c7b552336c4ecebf02bd8cedb32154ada Mon Sep 17 00:00:00 2001 From: Martti Tamm Date: Tue, 6 Feb 2024 14:16:08 +0200 Subject: [PATCH 5/6] Remove response dumping --- server/auth_oidc.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/server/auth_oidc.go b/server/auth_oidc.go index ab07dd66..f90a306b 100644 --- a/server/auth_oidc.go +++ b/server/auth_oidc.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "net/http" - "net/http/httputil" "net/url" "os" "strings" @@ -188,12 +187,6 @@ func (c *OidcConfig) isJwtActive(token string) bool { defer response.Body.Close() if response.StatusCode != http.StatusOK { - byteDump, err := httputil.DumpResponse(response, true) - if err != nil { - fmt.Printf("Failed to dump response: %s\n", err) - } else { - fmt.Print(string(byteDump)) - } fmt.Printf("[WARN] JWT introspection call gave non-200 HTTP status %d "+ "(thus JWT not active)\n", response.StatusCode) return false From 5487aa424d14704302f1065ea5dec1080c58366b Mon Sep 17 00:00:00 2001 From: Martti Tamm Date: Thu, 15 Feb 2024 10:58:50 +0200 Subject: [PATCH 6/6] Fix auth_test.go, minor err-check in auth_oidc.go --- server/auth_oidc.go | 7 +++++++ tests/auth/auth_test.go | 16 ++++++++-------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/server/auth_oidc.go b/server/auth_oidc.go index f90a306b..cd12fd30 100644 --- a/server/auth_oidc.go +++ b/server/auth_oidc.go @@ -159,6 +159,13 @@ func (c *OidcConfig) isJwtActive(token string) bool { c.remote.IntrospectionEndpoint, strings.NewReader(params)) + if err != nil { + fmt.Printf("[ERROR] Failed to create a new request for the OIDC "+ + "introspection endpoint (POST %s): %s\n", + c.remote.IntrospectionEndpoint, err) + return false + } + request.Header.Set("Content-Type", "application/x-www-form-urlencoded") if c.local.ClientId != "" && c.local.ClientSecret != "" { diff --git a/tests/auth/auth_test.go b/tests/auth/auth_test.go index 44ae3c1b..02c935ba 100644 --- a/tests/auth/auth_test.go +++ b/tests/auth/auth_test.go @@ -33,26 +33,26 @@ func TestBasicAuthFail(t *testing.T) { Id: "1", View: tes.TaskView_MINIMAL, }) - if err == nil || !strings.Contains(err.Error(), "STATUS CODE - 403") { + if err == nil || !strings.Contains(err.Error(), "STATUS CODE - 401") { t.Fatal("expected error") } _, err = fun.HTTP.ListTasks(ctx, &tes.ListTasksRequest{ View: tes.TaskView_MINIMAL, }) - if err == nil || !strings.Contains(err.Error(), "STATUS CODE - 403") { + if err == nil || !strings.Contains(err.Error(), "STATUS CODE - 401") { t.Fatal("expected error") } _, err = fun.HTTP.CreateTask(ctx, extask) - if err == nil || !strings.Contains(err.Error(), "STATUS CODE - 403") { + if err == nil || !strings.Contains(err.Error(), "STATUS CODE - 401") { t.Fatal("expected error") } _, err = fun.HTTP.CancelTask(ctx, &tes.CancelTaskRequest{ Id: "1", }) - if err == nil || !strings.Contains(err.Error(), "STATUS CODE - 403") { + if err == nil || !strings.Contains(err.Error(), "STATUS CODE - 401") { t.Fatal("expected error") } @@ -61,26 +61,26 @@ func TestBasicAuthFail(t *testing.T) { Id: "1", View: tes.TaskView_MINIMAL, }) - if err == nil || !strings.Contains(err.Error(), "PermissionDenied") { + if err == nil || !strings.Contains(err.Error(), "Unauthenticated") { t.Fatal("expected error") } _, err = fun.RPC.ListTasks(ctx, &tes.ListTasksRequest{ View: tes.TaskView_MINIMAL, }) - if err == nil || !strings.Contains(err.Error(), "PermissionDenied") { + if err == nil || !strings.Contains(err.Error(), "Unauthenticated") { t.Fatal("expected error") } _, err = fun.RPC.CreateTask(ctx, tests.HelloWorld()) - if err == nil || !strings.Contains(err.Error(), "PermissionDenied") { + if err == nil || !strings.Contains(err.Error(), "Unauthenticated") { t.Fatal("expected error") } _, err = fun.RPC.CancelTask(ctx, &tes.CancelTaskRequest{ Id: "1", }) - if err == nil || !strings.Contains(err.Error(), "PermissionDenied") { + if err == nil || !strings.Contains(err.Error(), "Unauthenticated") { t.Fatal("expected error") }