From f846b3dbebae057ff9d647bc4ed2cade094b4a42 Mon Sep 17 00:00:00 2001 From: Hugo Gonzalez Labrador Date: Tue, 1 Dec 2020 08:52:50 +0100 Subject: [PATCH 1/4] auth: add www-authenticate based on user agent --- .../http/services/dataprovider/_index.md | 14 ++- internal/http/interceptors/auth/auth.go | 87 ++++++++++++--- internal/http/interceptors/auth/auth_test.go | 101 ++++++++++++++++++ 3 files changed, 185 insertions(+), 17 deletions(-) create mode 100644 internal/http/interceptors/auth/auth_test.go diff --git a/docs/content/en/docs/config/http/services/dataprovider/_index.md b/docs/content/en/docs/config/http/services/dataprovider/_index.md index 39a32e3637..ac4426a971 100644 --- a/docs/content/en/docs/config/http/services/dataprovider/_index.md +++ b/docs/content/en/docs/config/http/services/dataprovider/_index.md @@ -9,7 +9,7 @@ description: > # _struct: config_ {{% dir name="prefix" type="string" default="data" %}} -The prefix to be used for this HTTP service [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/dataprovider/dataprovider.go#L39) +The prefix to be used for this HTTP service [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/dataprovider/dataprovider.go#L40) {{< highlight toml >}} [http.services.dataprovider] prefix = "data" @@ -17,7 +17,7 @@ prefix = "data" {{% /dir %}} {{% dir name="driver" type="string" default="localhome" %}} -The storage driver to be used. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/dataprovider/dataprovider.go#L40) +The storage driver to be used. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/dataprovider/dataprovider.go#L41) {{< highlight toml >}} [http.services.dataprovider] driver = "localhome" @@ -25,7 +25,7 @@ driver = "localhome" {{% /dir %}} {{% dir name="drivers" type="map[string]map[string]interface{}" default="localhome" %}} -The configuration for the storage driver [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/dataprovider/dataprovider.go#L41) +The configuration for the storage driver [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/dataprovider/dataprovider.go#L42) {{< highlight toml >}} [http.services.dataprovider.drivers.localhome] root = "/var/tmp/reva/" @@ -35,3 +35,11 @@ user_layout = "{{.Username}}" {{< /highlight >}} {{% /dir %}} +{{% dir name="data_txs" type="map[string]map[string]interface{}" default="simple" %}} +The configuration for the data tx protocols [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/dataprovider/dataprovider.go#L43) +{{< highlight toml >}} +[http.services.dataprovider.data_txs.simple] + +{{< /highlight >}} +{{% /dir %}} + diff --git a/internal/http/interceptors/auth/auth.go b/internal/http/interceptors/auth/auth.go index 055552a452..77ee475cac 100644 --- a/internal/http/interceptors/auth/auth.go +++ b/internal/http/interceptors/auth/auth.go @@ -21,6 +21,7 @@ package auth import ( "fmt" "net/http" + "strings" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -47,15 +48,16 @@ type config struct { Priority int `mapstructure:"priority"` GatewaySvc string `mapstructure:"gatewaysvc"` // TODO(jdf): Realm is optional, will be filled with request host if not given? - Realm string `mapstructure:"realm"` - CredentialChain []string `mapstructure:"credential_chain"` - CredentialStrategies map[string]map[string]interface{} `mapstructure:"credential_strategies"` - TokenStrategy string `mapstructure:"token_strategy"` - TokenStrategies map[string]map[string]interface{} `mapstructure:"token_strategies"` - TokenManager string `mapstructure:"token_manager"` - TokenManagers map[string]map[string]interface{} `mapstructure:"token_managers"` - TokenWriter string `mapstructure:"token_writer"` - TokenWriters map[string]map[string]interface{} `mapstructure:"token_writers"` + Realm string `mapstructure:"realm"` + CredentialsByUserAgent map[string]string `mapstructure:"credentials_by_user_agent"` + CredentialChain []string `mapstructure:"credential_chain"` + CredentialStrategies map[string]map[string]interface{} `mapstructure:"credential_strategies"` + TokenStrategy string `mapstructure:"token_strategy"` + TokenStrategies map[string]map[string]interface{} `mapstructure:"token_strategies"` + TokenManager string `mapstructure:"token_manager"` + TokenManagers map[string]map[string]interface{} `mapstructure:"token_managers"` + TokenWriter string `mapstructure:"token_writer"` + TokenWriters map[string]map[string]interface{} `mapstructure:"token_writers"` } func parseConfig(m map[string]interface{}) (*config, error) { @@ -93,8 +95,12 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err conf.CredentialChain = []string{"basic", "bearer"} } - credChain := []auth.CredentialStrategy{} - for i := range conf.CredentialChain { + if conf.CredentialsByUserAgent == nil { + conf.CredentialsByUserAgent = map[string]string{} + } + + credChain := map[string]auth.CredentialStrategy{} + for i, key := range conf.CredentialChain { f, ok := registry.NewCredentialFuncs[conf.CredentialChain[i]] if !ok { return nil, fmt.Errorf("credential strategy not found: %s", conf.CredentialChain[i]) @@ -104,7 +110,7 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err if err != nil { return nil, err } - credChain = append(credChain, credStrategy) + credChain[key] = credStrategy } g, ok := tokenregistry.NewTokenFuncs[conf.TokenStrategy] @@ -176,10 +182,43 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err if creds == nil { // TODO read realm from forwarded for header? // see https://github.com/stanvit/go-forwarded as middleware + // indicate all possible authentications to the client - for i := range credChain { - credChain[i].AddWWWAuthenticate(w, r, conf.Realm) + // if the CredentialsByUserAgent is set, forward only configured credential + // challenge. + + userAgent := r.UserAgent() + if len(conf.CredentialsByUserAgent) == 0 || userAgent == "" { + // set all available credentials challenges + for i := range credChain { + credChain[i].AddWWWAuthenticate(w, r, conf.Realm) + } + + } else { + // set credentials depending on user agent. + // if not match, set all available credentials. + var match bool + for k, cred := range conf.CredentialsByUserAgent { + if strings.Contains(userAgent, k) { + if challenge, ok := credChain[cred]; ok { + challenge.AddWWWAuthenticate(w, r, conf.Realm) + match = true + continue + } else { + // warm that configured credential is not loaded + log.Warn().Msgf("auth: configured user-agent credential is not loaded: %s", cred) + } + } + + } + // if no user agent is match, return all available challengues + if !match { + for i := range credChain { + credChain[i].AddWWWAuthenticate(w, r, conf.Realm) + } + } } + w.WriteHeader(http.StatusUnauthorized) return } @@ -247,3 +286,23 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err } return chain, nil } + +// applyWWWAuthenticate returns the WWW Authenticate challenges keys to use given an http request +// and available credentials. +func applyWWWAuthenticate(ua string, uam map[string]string, creds []string) []string { + if ua == "" || len(uam) == 0 { + return creds + } + + cred, ok := uam[ua] + if ok { + for _, v := range creds { + if v == cred { + return []string{cred} + } + } + return creds + } + + return nil +} diff --git a/internal/http/interceptors/auth/auth_test.go b/internal/http/interceptors/auth/auth_test.go new file mode 100644 index 0000000000..50f60025f3 --- /dev/null +++ b/internal/http/interceptors/auth/auth_test.go @@ -0,0 +1,101 @@ +// Copyright 2018-2020 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +package auth + +import ( + "testing" +) + +func TestApplyWWWAuthenticate(t *testing.T) { + type test struct { + userAgent string + userAgentMap map[string]string + availableCredentials []string + expected []string + } + + tests := []*test{ + // no user agent we return all available credentials + &test{ + userAgent: "", + userAgentMap: map[string]string{}, + availableCredentials: []string{}, + expected: []string{}, + }, + + // no user map we return all available credentials + &test{ + userAgent: "mirall", + userAgentMap: map[string]string{}, + availableCredentials: []string{"basic"}, + expected: []string{"basic"}, + }, + + // user agent set but no mapping set we return all credentials + &test{ + userAgent: "mirall", + userAgentMap: map[string]string{}, + availableCredentials: []string{"basic"}, + expected: []string{"basic"}, + }, + + // user mapping set to non available credential, we return all available + &test{ + userAgent: "mirall", + userAgentMap: map[string]string{"mirall": "notfound"}, + availableCredentials: []string{"basic", "bearer"}, + expected: []string{"basic", "bearer"}, + }, + + // user mapping set and we return only desired credential + &test{ + userAgent: "mirall", + userAgentMap: map[string]string{"mirall": "bearer"}, + availableCredentials: []string{"basic", "bearer"}, + expected: []string{"bearer"}, + }, + } + + for _, test := range tests { + got := applyWWWAuthenticate( + test.userAgent, + test.userAgentMap, + test.availableCredentials) + + if !match(got, test.expected) { + fail(t, got, test.expected) + } + } +} + +func match(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i, v := range a { + if v != b[i] { + return false + } + } + return true +} + +func fail(t *testing.T, got, expected []string) { + t.Fatalf("got: %+v expected: %+v", got, expected) +} From 4528d1a61d8a0ab178cffc7e2c00541d75d7b2f5 Mon Sep 17 00:00:00 2001 From: Hugo Gonzalez Labrador Date: Wed, 2 Dec 2020 08:33:42 +0100 Subject: [PATCH 2/4] add auth checks --- internal/http/interceptors/auth/auth.go | 55 +++++++------------------ 1 file changed, 14 insertions(+), 41 deletions(-) diff --git a/internal/http/interceptors/auth/auth.go b/internal/http/interceptors/auth/auth.go index 77ee475cac..b92af00950 100644 --- a/internal/http/interceptors/auth/auth.go +++ b/internal/http/interceptors/auth/auth.go @@ -21,7 +21,6 @@ package auth import ( "fmt" "net/http" - "strings" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -164,61 +163,35 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err return } - // check for token tkn := tokenStrategy.GetToken(r) if tkn == "" { log.Warn().Msg("core access token not set") + + credKeys := applyWWWAuthenticate(r.UserAgent(), conf.CredentialsByUserAgent, conf.CredentialChain) + + // obtain credentials (basic auth, bearer token, ...) based on user agent var creds *auth.Credentials - for i := range credChain { - creds, err = credChain[i].GetCredentials(w, r) + for _, k := range credKeys { + creds, err = credChain[k].GetCredentials(w, r) if err != nil { log.Debug().Err(err).Msg("error retrieving credentials") } + if creds != nil { log.Debug().Msgf("credentials obtained from credential strategy: type: %s, client_id: %s", creds.Type, creds.ClientID) break } } - if creds == nil { - // TODO read realm from forwarded for header? - // see https://github.com/stanvit/go-forwarded as middleware - - // indicate all possible authentications to the client - // if the CredentialsByUserAgent is set, forward only configured credential - // challenge. - - userAgent := r.UserAgent() - if len(conf.CredentialsByUserAgent) == 0 || userAgent == "" { - // set all available credentials challenges - for i := range credChain { - credChain[i].AddWWWAuthenticate(w, r, conf.Realm) - } - } else { - // set credentials depending on user agent. - // if not match, set all available credentials. - var match bool - for k, cred := range conf.CredentialsByUserAgent { - if strings.Contains(userAgent, k) { - if challenge, ok := credChain[cred]; ok { - challenge.AddWWWAuthenticate(w, r, conf.Realm) - match = true - continue - } else { - // warm that configured credential is not loaded - log.Warn().Msgf("auth: configured user-agent credential is not loaded: %s", cred) - } - } - - } - // if no user agent is match, return all available challengues - if !match { - for i := range credChain { - credChain[i].AddWWWAuthenticate(w, r, conf.Realm) - } + // if no credentials are found, reply with authentication challenge depending on user agent + if creds == nil { + for _, key := range credKeys { + if cred, ok := credChain[key]; ok { + cred.AddWWWAuthenticate(w, r, conf.Realm) + } else { + panic("auth credential strategy: " + key + "must have been loaded in init method") } } - w.WriteHeader(http.StatusUnauthorized) return } From edeccd9f90a4cfa99b29d5ce690b77120cc7d64f Mon Sep 17 00:00:00 2001 From: Hugo Gonzalez Labrador Date: Wed, 2 Dec 2020 08:33:47 +0100 Subject: [PATCH 3/4] add changelog --- changelog/unreleased/auth-by-user-agent.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 changelog/unreleased/auth-by-user-agent.md diff --git a/changelog/unreleased/auth-by-user-agent.md b/changelog/unreleased/auth-by-user-agent.md new file mode 100644 index 0000000000..b02ea4c677 --- /dev/null +++ b/changelog/unreleased/auth-by-user-agent.md @@ -0,0 +1,12 @@ +Enhancement: Add auth protocol based on user agent + +Previously, all available credential challenges are given to the client, +for example, basic auth, bearer token, etc ... +Different clients have different priorities to use one method or another, +and before it was not possible to force a client to use one method without +having a side effect on other clients. + +This PR adds the functionality to target a specific auth protocol based +on the user agent HTTP header. + +https://github.com/cs3org/reva/pull/1350 From befa4a5708b69487a9b9c245e56925e052173df0 Mon Sep 17 00:00:00 2001 From: Hugo Gonzalez Labrador Date: Wed, 2 Dec 2020 14:42:37 +0100 Subject: [PATCH 4/4] add missing test --- .../grpc/services/storageprovider/_index.md | 18 +++++++++--------- internal/http/interceptors/auth/auth.go | 12 ++++++------ internal/http/interceptors/auth/auth_test.go | 16 ++++++++++++---- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/docs/content/en/docs/config/grpc/services/storageprovider/_index.md b/docs/content/en/docs/config/grpc/services/storageprovider/_index.md index 3dde1cd5ab..6e6b36f4c3 100644 --- a/docs/content/en/docs/config/grpc/services/storageprovider/_index.md +++ b/docs/content/en/docs/config/grpc/services/storageprovider/_index.md @@ -9,7 +9,7 @@ description: > # _struct: config_ {{% dir name="mount_path" type="string" default="/" %}} -The path where the file system would be mounted. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L53) +The path where the file system would be mounted. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L52) {{< highlight toml >}} [grpc.services.storageprovider] mount_path = "/" @@ -17,7 +17,7 @@ mount_path = "/" {{% /dir %}} {{% dir name="mount_id" type="string" default="-" %}} -The ID of the mounted file system. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L54) +The ID of the mounted file system. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L53) {{< highlight toml >}} [grpc.services.storageprovider] mount_id = "-" @@ -25,7 +25,7 @@ mount_id = "-" {{% /dir %}} {{% dir name="driver" type="string" default="localhome" %}} -The storage driver to be used. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L55) +The storage driver to be used. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L54) {{< highlight toml >}} [grpc.services.storageprovider] driver = "localhome" @@ -33,7 +33,7 @@ driver = "localhome" {{% /dir %}} {{% dir name="drivers" type="map[string]map[string]interface{}" default="localhome" %}} - [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L56) + [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L55) {{< highlight toml >}} [grpc.services.storageprovider.drivers.localhome] root = "/var/tmp/reva/" @@ -44,7 +44,7 @@ user_layout = "{{.Username}}" {{% /dir %}} {{% dir name="tmp_folder" type="string" default="/var/tmp" %}} -Path to temporary folder. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L57) +Path to temporary folder. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L56) {{< highlight toml >}} [grpc.services.storageprovider] tmp_folder = "/var/tmp" @@ -52,7 +52,7 @@ tmp_folder = "/var/tmp" {{% /dir %}} {{% dir name="data_server_url" type="string" default="http://localhost/data" %}} -The URL for the data server. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L58) +The URL for the data server. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L57) {{< highlight toml >}} [grpc.services.storageprovider] data_server_url = "http://localhost/data" @@ -60,7 +60,7 @@ data_server_url = "http://localhost/data" {{% /dir %}} {{% dir name="expose_data_server" type="bool" default=false %}} -Whether to expose data server. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L59) +Whether to expose data server. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L58) {{< highlight toml >}} [grpc.services.storageprovider] expose_data_server = false @@ -68,7 +68,7 @@ expose_data_server = false {{% /dir %}} {{% dir name="available_checksums" type="map[string]uint32" default=nil %}} -List of available checksums. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L60) +List of available checksums. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L59) {{< highlight toml >}} [grpc.services.storageprovider] available_checksums = nil @@ -76,7 +76,7 @@ available_checksums = nil {{% /dir %}} {{% dir name="mimetypes" type="map[string]string" default=nil %}} -List of supported mime types and corresponding file extensions. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L61) +List of supported mime types and corresponding file extensions. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L60) {{< highlight toml >}} [grpc.services.storageprovider] mimetypes = nil diff --git a/internal/http/interceptors/auth/auth.go b/internal/http/interceptors/auth/auth.go index b92af00950..42d6808b45 100644 --- a/internal/http/interceptors/auth/auth.go +++ b/internal/http/interceptors/auth/auth.go @@ -167,11 +167,11 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err if tkn == "" { log.Warn().Msg("core access token not set") - credKeys := applyWWWAuthenticate(r.UserAgent(), conf.CredentialsByUserAgent, conf.CredentialChain) + userAgentCredKeys := getCredsForUserAgent(r.UserAgent(), conf.CredentialsByUserAgent, conf.CredentialChain) // obtain credentials (basic auth, bearer token, ...) based on user agent var creds *auth.Credentials - for _, k := range credKeys { + for _, k := range userAgentCredKeys { creds, err = credChain[k].GetCredentials(w, r) if err != nil { log.Debug().Err(err).Msg("error retrieving credentials") @@ -185,7 +185,7 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err // if no credentials are found, reply with authentication challenge depending on user agent if creds == nil { - for _, key := range credKeys { + for _, key := range userAgentCredKeys { if cred, ok := credChain[key]; ok { cred.AddWWWAuthenticate(w, r, conf.Realm) } else { @@ -260,9 +260,9 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err return chain, nil } -// applyWWWAuthenticate returns the WWW Authenticate challenges keys to use given an http request +// getCredsForUserAgent returns the WWW Authenticate challenges keys to use given an http request // and available credentials. -func applyWWWAuthenticate(ua string, uam map[string]string, creds []string) []string { +func getCredsForUserAgent(ua string, uam map[string]string, creds []string) []string { if ua == "" || len(uam) == 0 { return creds } @@ -277,5 +277,5 @@ func applyWWWAuthenticate(ua string, uam map[string]string, creds []string) []st return creds } - return nil + return creds } diff --git a/internal/http/interceptors/auth/auth_test.go b/internal/http/interceptors/auth/auth_test.go index 50f60025f3..bce076806c 100644 --- a/internal/http/interceptors/auth/auth_test.go +++ b/internal/http/interceptors/auth/auth_test.go @@ -22,7 +22,7 @@ import ( "testing" ) -func TestApplyWWWAuthenticate(t *testing.T) { +func TestGetCredsForUserAgent(t *testing.T) { type test struct { userAgent string userAgentMap map[string]string @@ -35,8 +35,16 @@ func TestApplyWWWAuthenticate(t *testing.T) { &test{ userAgent: "", userAgentMap: map[string]string{}, - availableCredentials: []string{}, - expected: []string{}, + availableCredentials: []string{"basic"}, + expected: []string{"basic"}, + }, + + // map set but user agent not in map + &test{ + userAgent: "curl", + userAgentMap: map[string]string{"mirall": "basic"}, + availableCredentials: []string{"basic", "bearer"}, + expected: []string{"basic", "bearer"}, }, // no user map we return all available credentials @@ -73,7 +81,7 @@ func TestApplyWWWAuthenticate(t *testing.T) { } for _, test := range tests { - got := applyWWWAuthenticate( + got := getCredsForUserAgent( test.userAgent, test.userAgentMap, test.availableCredentials)