Skip to content

Commit

Permalink
Fix authorization webhook implementation and add tests (#537)
Browse files Browse the repository at this point in the history
* Fix authz webhook not being called

Signed-off-by: Nikos Tsipinakis <[email protected]>

* Add codecov support for coverage reporting

Signed-off-by: Nikos Tsipinakis <[email protected]>

* Unify authz configuration with the other methods

Authz webhook configuration will now be placed under the 'webhook' key
to be in line with the other authentication methods.

Signed-off-by: Nikos Tsipinakis <[email protected]>

* Add authz support to testauthconfig server

Signed-off-by: Nikos Tsipinakis <[email protected]>

* Test authz in integration

Signed-off-by: Nikos Tsipinakis <[email protected]>

---------

Signed-off-by: Nikos Tsipinakis <[email protected]>
  • Loading branch information
tsipinakis authored Oct 7, 2023
1 parent 13dbb35 commit ad1233a
Show file tree
Hide file tree
Showing 8 changed files with 284 additions and 70 deletions.
6 changes: 5 additions & 1 deletion .github/workflows/pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ jobs:
- name: Run go test
run: |
set -euo pipefail
go test -json -p 1 -v ./... 2>&1 | tee /tmp/gotest.log
go test -coverprofile=coverage.txt -covermode=atomic -json -p 1 -v ./... 2>&1 | tee /tmp/gotest.log
- name: Format log output
if: always()
run: |
Expand All @@ -100,3 +100,7 @@ jobs:
name: test-log
path: /tmp/gotest.log
if-no-files-found: error
- name: Upload coverage reports to Codecov
uses: codecov/codecov-action@v3
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
94 changes: 52 additions & 42 deletions cmd/containerssh-testauthconfigserver/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
// This OpenAPI document describes the API endpoints that are required for implementing an authentication
// and configuration server for ContainerSSH. (See https://github.com/containerssh/libcontainerssh for details.)
//
// Schemes: http, https
// Host: localhost
// BasePath: /
// Version: 0.5.0
// Schemes: http, https
// Host: localhost
// BasePath: /
// Version: 0.5.0
//
// Consumes:
// - application/json
// Consumes:
// - application/json
//
// Produces:
// - application/json
// Produces:
// - application/json
//
// swagger:meta
package main
Expand Down Expand Up @@ -40,73 +40,79 @@ type authHandler struct {

// swagger:operation POST /password Authentication authPassword
//
// Password authentication
// # Password authentication
//
// ---
// parameters:
// - name: request
// in: body
// description: The authentication request
// required: true
// schema:
// - name: request
// in: body
// description: The authentication request
// required: true
// schema:
// "$ref": "#/definitions/PasswordAuthRequest"
//
// responses:
// "200":
// "$ref": "#/responses/AuthResponse"
//
// "200":
// "$ref": "#/responses/AuthResponse"
func (a *authHandler) OnPassword(meta metadata.ConnectionAuthPendingMetadata, Password []byte) (
bool,
metadata.ConnectionAuthenticatedMetadata,
error,
) {
if os.Getenv("CONTAINERSSH_ALLOW_ALL") == "1" ||
meta.Username == "foo" ||
meta.Username == "busybox" {
return true, meta.Authenticated(meta.Username), nil
meta.Username == "busybox" || meta.Username == "foonoauthz" {
return string(Password) == "bar", meta.Authenticated(meta.Username), nil
}
return false, meta.AuthFailed(), nil
}

// swagger:operation POST /pubkey Authentication authPubKey
//
// Public key authentication
// # Public key authentication
//
// ---
// parameters:
// - name: request
// in: body
// description: The authentication request
// required: true
// schema:
// - name: request
// in: body
// description: The authentication request
// required: true
// schema:
// "$ref": "#/definitions/PublicKeyAuthRequest"
//
// responses:
// "200":
// "$ref": "#/responses/AuthResponse"
//
// "200":
// "$ref": "#/responses/AuthResponse"
func (a *authHandler) OnPubKey(meta metadata.ConnectionAuthPendingMetadata, publicKey publicAuth.PublicKey) (
bool,
metadata.ConnectionAuthenticatedMetadata,
error,
) {
if meta.Username == "foo" || meta.Username == "busybox" {
return true, meta.Authenticated(meta.Username), nil
if meta.Username == "foo" || meta.Username == "busybox" || meta.Username == "foonoauthz" {
return false, meta.Authenticated(meta.Username), nil
}
return false, meta.AuthFailed(), nil
}

// swagger:operation POST /authz Authentication authz
//
// Authorization
// # Authorization
//
// ---
// parameters:
// - name: request
// in: body
// description: The authorization request
// required: true
// schema:
// - name: request
// in: body
// description: The authorization request
// required: true
// schema:
// "$ref": "#/definitions/AuthorizationRequest"
//
// responses:
// "200":
// "$ref": "#/responses/AuthResponse"
//
// "200":
// "$ref": "#/responses/AuthResponse"
func (a *authHandler) OnAuthorization(meta metadata.ConnectionAuthenticatedMetadata) (
bool,
metadata.ConnectionAuthenticatedMetadata,
Expand All @@ -127,14 +133,16 @@ type configHandler struct {
//
// ---
// parameters:
// - name: request
// in: body
// description: The configuration request
// schema:
// - name: request
// in: body
// description: The configuration request
// schema:
// "$ref": "#/definitions/ConfigRequest"
//
// responses:
// "200":
// "$ref": "#/responses/ConfigResponse"
//
// "200":
// "$ref": "#/responses/ConfigResponse"
func (c *configHandler) OnConfig(request config.Request) (config.AppConfig, error) {
cfg := config.AppConfig{}

Expand All @@ -156,6 +164,8 @@ type handler struct {

func (h *handler) ServeHTTP(writer goHttp.ResponseWriter, request *goHttp.Request) {
switch request.URL.Path {
case "/authz":
fallthrough
case "/password":
fallthrough
case "/pubkey":
Expand Down
4 changes: 2 additions & 2 deletions config/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ func (o *AuthGenericConfig) Validate() error {
type AuthzConfig struct {
Method AuthzMethod `json:"method" yaml:"method" default:""`

AuthWebhookClientConfig `json:",inline" yaml:",inline"`
Webhook AuthWebhookClientConfig `json:"webhook" yaml:"webhook"`
}

// Validate validates the authorization configuration.
Expand All @@ -684,7 +684,7 @@ func (k *AuthzConfig) Validate() error {
case AuthzMethodDisabled:
return nil
case AuthzMethodWebhook:
return wrap(k.AuthWebhookClientConfig.Validate(), "webhook")
return wrap(k.Webhook.Validate(), "webhook")
default:
return newError("method", "BUG: invalid value for method for authorization: %s", k.Method)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/auth/authentication_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func NewAuthorizationProvider(
case config.AuthzMethodDisabled:
return nil, nil, nil
case config.AuthzMethodWebhook:
cli, err := NewWebhookClient(AuthenticationTypeAuthz, cfg.AuthWebhookClientConfig, logger, metrics)
cli, err := NewWebhookClient(AuthenticationTypeAuthz, cfg.Webhook, logger, metrics)
return cli, nil, err
default:
return nil, nil, fmt.Errorf("unsupported method: %s", cfg.Method)
Expand Down
13 changes: 9 additions & 4 deletions internal/auth/webhook_client_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,13 @@ func (client *webhookClient) Authorize(
client.logger.Debug(err)
return &webhookClientContext{meta.AuthFailed(), false, err}
}
return client.processAuthzWithRetry(meta)

url := client.endpoint + "/authz"
authzRequest := auth.AuthorizationRequest{
ConnectionAuthenticatedMetadata: meta,
}

return client.processAuthzWithRetry(meta, url, authzRequest)
}

func (client *webhookClient) Password(
Expand Down Expand Up @@ -268,10 +274,9 @@ func (client *webhookClient) authServerRequest(endpoint string, requestObject in

func (client *webhookClient) processAuthzWithRetry(
meta metadata.ConnectionAuthenticatedMetadata,
url string,
authzRequest interface{},
) AuthenticationContext {
url := client.endpoint + "/authz"
authzRequest := auth.AuthorizationRequest{}

ctx, cancel := context.WithTimeout(context.Background(), client.timeout)
defer cancel()
var lastError error
Expand Down
31 changes: 31 additions & 0 deletions internal/auth/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,19 @@ func (h *handler) OnAuthorization(meta metadata.ConnectionAuthenticatedMetadata)
metadata.ConnectionAuthenticatedMetadata,
error,
) {
if meta.RemoteAddress.IP.String() != "127.0.0.1" {
return false, meta.AuthFailed(), fmt.Errorf("invalid IP: %s", meta.RemoteAddress.IP.String())
}
if meta.ConnectionID != "0123456789ABCDEF" {
return false, meta.AuthFailed(), fmt.Errorf("invalid connection ID: %s", meta.ConnectionID)
}
if meta.AuthenticatedUsername == "foo" {
return true, meta.AuthFailed(), nil
}
if meta.Username == "crash" {
// Simulate a database failure
return false, meta.AuthFailed(), fmt.Errorf("database error")
}
return false, meta.AuthFailed(), nil
}

Expand Down Expand Up @@ -143,6 +156,24 @@ func TestAuth(t *testing.T) {
)
assert.NotEqual(t, nil, authenticationContext.Error())
assert.Equal(t, false, authenticationContext.Success())

authenticationContext = client.Authorize(
metadata.NewTestAuthenticatingMetadata("foo").Authenticated("foo"),
)
assert.Equal(t, nil, authenticationContext.Error())
assert.Equal(t, true, authenticationContext.Success())

authenticationContext = client.Authorize(
metadata.NewTestAuthenticatingMetadata("foo").Authenticated("foonoauthz"),
)
assert.Equal(t, nil, authenticationContext.Error())
assert.Equal(t, false, authenticationContext.Success())

authenticationContext = client.Authorize(
metadata.NewTestAuthenticatingMetadata("crash").Authenticated("crash"),
)
assert.NotEqual(t, nil, authenticationContext.Error())
assert.Equal(t, false, authenticationContext.Success())
},
)
}
Expand Down
Loading

0 comments on commit ad1233a

Please sign in to comment.