Skip to content

Commit

Permalink
Add default auth query params (#465)
Browse files Browse the repository at this point in the history
Add default allowed query param option
  • Loading branch information
p53 committed May 20, 2024
1 parent befeb07 commit 679ec15
Show file tree
Hide file tree
Showing 12 changed files with 196 additions and 7 deletions.
1 change: 1 addition & 0 deletions docs/content/configuration/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ weight: 2
| --response-headers value | custom headers to added to the http response key=value | | PROXY_RESPONSE_HEADERS
| --custom-http-methods | list of additional non-standard http methods | |
| --allowed-query-params | allowed query params, sent to IDP key=optional value | |
| --default-allowed-query-params | default allowed query params, sent to IDP key=required-value | |
| --enable-self-signed-tls | create self signed certificates for the proxy | false | PROXY_ENABLE_SELF_SIGNED_TLS
| --self-signed-tls-hostnames value | a list of hostnames to place on the self-signed certificate | |
| --self-signed-tls-expiration value | the expiration of the certificate before rotation | 3h0m0s | PROXY_SELF_SIGNED_TLS_EXPIRATION
Expand Down
22 changes: 22 additions & 0 deletions docs/content/userguide/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ This example will allow passing `myparam` and `yourparam` with any value to IDP:
```

yaml example:

```yaml
allowed-query-params:
myparam: ""
Expand All @@ -319,12 +320,33 @@ This example will allow passing `myparam` and `yourparam` only with specified va
```

yaml example:

```yaml
allowed-query-params:
myparam: "myvalue"
yourparam: "yourvalue"
```

If you would like to have defaults for your parameters, there is `default-allowed-query-params` option available, these values
will be used only when there are no params specified in url:

**NOTE**: Params present in default query params must be allowed by allowed-query-params option

cli example:

```bash
--default-allowed-query-params="myparam=myvalue" \
--default-allowed-query-params="yourparam=yourvalue"
```

yaml example:

```yaml
default-allowed-query-params:
myparam: "myvalue"
yourparam: "yourvalue"
```

## TCP proxy with HTTP CONNECT

You can protect your TCP services with gogatekeeper by adding `CONNECT` HTTP method to list of `custom-http-methods`. On client side you will need to pass of course token in `Authorization` header (righ now there are few clients which could make HTTP connect with `Bearer` token and then forward tcp, e.g. gost proxy - but only in static way, some IDE provide HTTP CONNECT functionality for db connectors but only with `Basic` authentication, we would like to add this functionality to gatekeeper in future). This setup will authenticate connection at start and will create tunnel to your backend service. Please use with care and ensure that it allows connection only to intended services, otherwise it can be missused for various attacks.
Expand Down
4 changes: 4 additions & 0 deletions pkg/apperrors/apperrors.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,4 +148,8 @@ var (
ErrHmacRequiresEncKey = errors.New("enable-hmac requires encryption key")
ErrPostLogoutRedirectURIRequiresIDToken = errors.New("post logout redirect uri requires id token, enable id token cookie")
ErrAllowedQueryParamsWithNoRedirects = errors.New("allowed-query-params are not valid with noredirects=true")
ErrDefaultAllowedQueryParamEmpty = errors.New("default-allowed-query-params value cannot be empty")
ErrTooManyDefaultAllowedQueryParams = errors.New("you have more default query params than allowed query params")
ErrMissingDefaultQueryParamInAllowed = errors.New("param is present in default query params but missing in allowed")
ErrDefaultQueryParamNotAllowed = errors.New("default query param is not in allowed query params")
)
1 change: 1 addition & 0 deletions pkg/config/core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type Configs interface {
GetMatchClaims() map[string]string
GetTags() map[string]string
GetAllowedQueryParams() map[string]string
GetDefaultAllowedQueryParams() map[string]string
}

type CommonConfig struct{}
Expand Down
7 changes: 7 additions & 0 deletions pkg/google/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ type Config struct {
CustomHTTPMethods []string `json:"custom-http-methods" usage:"list of additional non-standard http methods" yaml:"custom-http-methods"`
// Allowed Query Params sent to IDP
AllowedQueryParams map[string]string `json:"allowed-query-params" usage:"allowed query params, sent to IDP key=optional value" yaml:"allowed-query-params"`
// Default Allowed Query Params
DefaultAllowedQueryParams map[string]string `json:"default-allowed-query-params" usage:"default allowed query params, sent to IDP key=value" yaml:"default-allowed-query-params"`

// EnableSelfSignedTLS indicates we should create a self-signed ceritificate for the service
EnabledSelfSignedTLS bool `env:"ENABLE_SELF_SIGNED_TLS" json:"enable-self-signed-tls" usage:"create self signed certificates for the proxy" yaml:"enable-self-signed-tls"`
Expand Down Expand Up @@ -335,6 +337,7 @@ func NewDefaultConfig() *Config {
HTTPOnlyCookie: true,
Headers: make(map[string]string),
AllowedQueryParams: make(map[string]string),
DefaultAllowedQueryParams: make(map[string]string),
LetsEncryptCacheDir: "./cache/",
MatchClaims: make(map[string]string),
MaxIdleConns: 100,
Expand Down Expand Up @@ -397,6 +400,10 @@ func (r *Config) GetAllowedQueryParams() map[string]string {
return r.AllowedQueryParams
}

func (r *Config) GetDefaultAllowedQueryParams() map[string]string {
return r.DefaultAllowedQueryParams
}

// readConfigFile reads and parses the configuration file
func (r *Config) ReadConfigFile(filename string) error {
content, err := os.ReadFile(filename)
Expand Down
24 changes: 23 additions & 1 deletion pkg/keycloak/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ type Config struct {
CustomHTTPMethods []string `json:"custom-http-methods" usage:"list of additional non-standard http methods" yaml:"custom-http-methods"`
// Allowed Query Params sent to IDP
AllowedQueryParams map[string]string `json:"allowed-query-params" usage:"allowed query params, sent to IDP key=optional value" yaml:"allowed-query-params"`
// Default Allowed Query Params
DefaultAllowedQueryParams map[string]string `json:"default-allowed-query-params" usage:"default allowed query params, sent to IDP key=value" yaml:"default-allowed-query-params"`

// EnableSelfSignedTLS indicates we should create a self-signed ceritificate for the service
EnabledSelfSignedTLS bool `env:"ENABLE_SELF_SIGNED_TLS" json:"enable-self-signed-tls" usage:"create self signed certificates for the proxy" yaml:"enable-self-signed-tls"`
Expand Down Expand Up @@ -351,6 +353,7 @@ func NewDefaultConfig() *Config {
HTTPOnlyCookie: true,
Headers: make(map[string]string),
AllowedQueryParams: make(map[string]string),
DefaultAllowedQueryParams: make(map[string]string),
LetsEncryptCacheDir: "./cache/",
MatchClaims: make(map[string]string),
MaxIdleConns: 100,
Expand Down Expand Up @@ -413,6 +416,10 @@ func (r *Config) GetAllowedQueryParams() map[string]string {
return r.AllowedQueryParams
}

func (r *Config) GetDefaultAllowedQueryParams() map[string]string {
return r.DefaultAllowedQueryParams
}

// readConfigFile reads and parses the configuration file
func (r *Config) ReadConfigFile(filename string) error {
content, err := os.ReadFile(filename)
Expand Down Expand Up @@ -1015,8 +1022,23 @@ func (r *Config) isPostLogoutRedirectURIValid() error {
}

func (r *Config) isAllowedQueryParamsValid() error {
if len(r.AllowedQueryParams) > 0 && r.NoRedirects {
if (len(r.AllowedQueryParams) > 0 || len(r.DefaultAllowedQueryParams) > 0) && r.NoRedirects {
return apperrors.ErrAllowedQueryParamsWithNoRedirects
}
if len(r.DefaultAllowedQueryParams) > len(r.AllowedQueryParams) {
return apperrors.ErrTooManyDefaultAllowedQueryParams
}
for k, val := range r.DefaultAllowedQueryParams {
if val == "" {
return apperrors.ErrDefaultAllowedQueryParamEmpty
}
allowedVal, ok := r.AllowedQueryParams[k]
if !ok {
return apperrors.ErrMissingDefaultQueryParamInAllowed
}
if allowedVal != "" && val != allowedVal {
return apperrors.ErrDefaultQueryParamNotAllowed
}
}
return nil
}
69 changes: 63 additions & 6 deletions pkg/keycloak/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ limitations under the License.
package config

import (
"errors"
"fmt"
"math/rand"
"net/url"
"os"
"testing"
"time"

"github.com/gogatekeeper/gatekeeper/pkg/apperrors"
"github.com/gogatekeeper/gatekeeper/pkg/authorization"
"github.com/gogatekeeper/gatekeeper/pkg/config/core"
"github.com/gogatekeeper/gatekeeper/pkg/constant"
Expand Down Expand Up @@ -2480,9 +2482,10 @@ func TestIsPostLogoutRedirectURIValid(t *testing.T) {

func TestIsAllowedQueryParamsValid(t *testing.T) {
testCases := []struct {
Name string
Config *Config
Valid bool
Name string
Config *Config
Valid bool
ExptectedError error
}{
{
Name: "AllowedQueryParamsValidValid",
Expand All @@ -2498,7 +2501,56 @@ func TestIsAllowedQueryParamsValid(t *testing.T) {
AllowedQueryParams: map[string]string{"this": "that"},
NoRedirects: true,
},
Valid: false,
Valid: false,
ExptectedError: apperrors.ErrAllowedQueryParamsWithNoRedirects,
},
{
Name: "DefaultAllowedQueryParamsValidWithNoRedirectsInvalid",
Config: &Config{
DefaultAllowedQueryParams: map[string]string{"this": "that"},
NoRedirects: true,
},
Valid: false,
ExptectedError: apperrors.ErrAllowedQueryParamsWithNoRedirects,
},
{
Name: "DefaultAllowedQueryParamsWithEmptyValueInvalid",
Config: &Config{
AllowedQueryParams: map[string]string{"this": "that"},
DefaultAllowedQueryParams: map[string]string{"this": ""},
NoRedirects: false,
},
Valid: false,
ExptectedError: apperrors.ErrDefaultAllowedQueryParamEmpty,
},
{
Name: "MoreDefaultParamsThanAllowedInvalid",
Config: &Config{
AllowedQueryParams: map[string]string{"this": "that"},
DefaultAllowedQueryParams: map[string]string{"this": "that", "thiiiis": "thoose"},
NoRedirects: false,
},
Valid: false,
ExptectedError: apperrors.ErrTooManyDefaultAllowedQueryParams,
},
{
Name: "DefaultParamsDoesNotMatchAllowedInvalid",
Config: &Config{
AllowedQueryParams: map[string]string{"this": "that"},
DefaultAllowedQueryParams: map[string]string{"this": "thatt"},
NoRedirects: false,
},
Valid: false,
ExptectedError: apperrors.ErrDefaultQueryParamNotAllowed,
},
{
Name: "DefaultParamsDoesNotMatchAllowedValid",
Config: &Config{
AllowedQueryParams: map[string]string{"this": ""},
DefaultAllowedQueryParams: map[string]string{"this": "thatt"},
NoRedirects: false,
},
Valid: true,
},
}

Expand All @@ -2508,8 +2560,13 @@ func TestIsAllowedQueryParamsValid(t *testing.T) {
testCase.Name,
func(t *testing.T) {
err := testCase.Config.isAllowedQueryParamsValid()
if err != nil && testCase.Valid {
t.Fatalf("Expected test not to fail")
if err != nil {
if testCase.Valid {
t.Fatalf("Expected test not to fail")
}
if !errors.Is(err, testCase.ExptectedError) {
t.Fatalf("Exptected %s, got %s", testCase.ExptectedError, err)
}
}

if err == nil && !testCase.Valid {
Expand Down
8 changes: 8 additions & 0 deletions pkg/keycloak/proxy/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ func oauthAuthorizationHandler(
getRedirectionURL func(wrt http.ResponseWriter, req *http.Request) string,
customSignInPage func(wrt http.ResponseWriter, authURL string),
allowedQueryParams map[string]string,
defaultAllowedQueryParams map[string]string,
) func(wrt http.ResponseWriter, req *http.Request) {
return func(wrt http.ResponseWriter, req *http.Request) {
if skipTokenVerification {
Expand Down Expand Up @@ -180,6 +181,13 @@ func oauthAuthorizationHandler(
authCodeOptions,
oauth2.SetAuthURLParam(key, param),
)
} else {
if val, ok := defaultAllowedQueryParams[key]; ok {
authCodeOptions = append(
authCodeOptions,
oauth2.SetAuthURLParam(key, val),
)
}
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/keycloak/proxy/misc.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ func redirectToAuthorization(
baseURI string,
oAuthURI string,
allowedQueryParams map[string]string,
defaultAllowedQueryParams map[string]string,
) func(wrt http.ResponseWriter, req *http.Request) context.Context {
return func(wrt http.ResponseWriter, req *http.Request) context.Context {
if noRedirects {
Expand All @@ -204,6 +205,10 @@ func redirectToAuthorization(
}
}
query += fmt.Sprintf("&%s=%s", key, param)
} else {
if val, ok := defaultAllowedQueryParams[key]; ok {
query += fmt.Sprintf("&%s=%s", key, val)
}
}
}
authQuery += query
Expand Down
2 changes: 2 additions & 0 deletions pkg/keycloak/proxy/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ func (r *OauthProxy) CreateReverseProxy() error {
r.Config.BaseURI,
r.Config.OAuthURI,
r.Config.AllowedQueryParams,
r.Config.DefaultAllowedQueryParams,
)

if r.Config.EnableHmac {
Expand Down Expand Up @@ -534,6 +535,7 @@ func (r *OauthProxy) CreateReverseProxy() error {
r.getRedirectionURL,
r.customSignInPage,
r.Config.AllowedQueryParams,
r.Config.DefaultAllowedQueryParams,
)

// step: add the routing for oauth
Expand Down
8 changes: 8 additions & 0 deletions pkg/proxy/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,14 @@ func parseCLIOptions(cliCtx *cli.Context, config core.Configs) error {
utils.MergeMaps(config.GetAllowedQueryParams(), headers)
}

if cliCtx.IsSet("default-allowed-query-params") {
headers, err := utils.DecodeKeyPairs(cliCtx.StringSlice("default-allowed-query-params"))
if err != nil {
return err
}
utils.MergeMaps(config.GetDefaultAllowedQueryParams(), headers)
}

if cliCtx.IsSet("resources") {
for _, x := range cliCtx.StringSlice("resources") {
resource, err := authorization.NewResource().Parse(x)
Expand Down
52 changes: 52 additions & 0 deletions pkg/testsuite/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,58 @@ func TestAuthorizationURL(t *testing.T) {
},
},
},
{
Name: "TestQueryParamsWithDefaultValueAllowedAny",
ProxySettings: func(conf *config.Config) {
conf.NoRedirects = false
conf.AllowedQueryParams = map[string]string{
"test": "",
"test1": "test",
}
conf.DefaultAllowedQueryParams = map[string]string{
"test": "yes",
}
},
ExecutionSettings: []fakeRequest{
{
URI: "/admin?test1=test",
Redirects: true,
ExpectedHeadersValidator: map[string]func(*testing.T, *config.Config, string){
"Location": func(t *testing.T, c *config.Config, value string) {
assert.Contains(t, value, "test1=test")
assert.Contains(t, value, "test=yes")
},
},
ExpectedCode: http.StatusSeeOther,
},
},
},
{
Name: "TestQueryParamsWithDefaultValueAllowedSpecific",
ProxySettings: func(conf *config.Config) {
conf.NoRedirects = false
conf.AllowedQueryParams = map[string]string{
"test": "yes",
"test1": "test",
}
conf.DefaultAllowedQueryParams = map[string]string{
"test": "yes",
}
},
ExecutionSettings: []fakeRequest{
{
URI: "/admin?test1=test",
Redirects: true,
ExpectedHeadersValidator: map[string]func(*testing.T, *config.Config, string){
"Location": func(t *testing.T, c *config.Config, value string) {
assert.Contains(t, value, "test1=test")
assert.Contains(t, value, "test=yes")
},
},
ExpectedCode: http.StatusSeeOther,
},
},
},
}

for _, testCase := range testCases {
Expand Down

0 comments on commit 679ec15

Please sign in to comment.