Skip to content

Commit

Permalink
define a single auth per role directly within role
Browse files Browse the repository at this point in the history
  • Loading branch information
qdm12 committed Sep 11, 2024
1 parent 11f67e8 commit 47113b6
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 169 deletions.
22 changes: 3 additions & 19 deletions internal/server/middlewares/auth/configfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,30 +30,14 @@ func Test_Read(t *testing.T) {
| ~~~~~~~ missing field`,
},
"filled_settings": {
fileContent: `[[auths]]
name = "abc"
method = "none"
[[auths]]
name = "xyz"
# comments are ignored
method = "oauth2"
[[roles]]
fileContent: `[[roles]]
name = "public"
auths = ["abc"]
auth = "none"
routes = ["GET /v1/vpn/status", "PUT /v1/vpn/status"]`,
settings: Settings{
Auths: []Auth{{
Name: "abc",
Method: MethodNone,
}, {
Name: "xyz",
Method: "oauth2",
}},
Roles: []Role{{
Name: "public",
Auths: []string{"abc"},
Auth: AuthNone,
Routes: []string{"GET /v1/vpn/status", "PUT /v1/vpn/status"},
}},
},
Expand Down
4 changes: 0 additions & 4 deletions internal/server/middlewares/auth/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@ func andStrings(strings []string) (result string) {
return joinStrings(strings, "and")
}

func orStrings(strings []string) (result string) {
return joinStrings(strings, "or")
}

func joinStrings(strings []string, lastJoin string) (result string) {
if len(strings) == 0 {
return ""
Expand Down
56 changes: 22 additions & 34 deletions internal/server/middlewares/auth/lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package auth

import (
"fmt"

"golang.org/x/exp/maps"
)

type internalRole struct {
Expand All @@ -12,43 +10,33 @@ type internalRole struct {
}

func settingsToLookupMap(settings Settings) (routeToRoles map[string][]internalRole, err error) {
authNameToChecker := make(map[string]authorizationChecker, len(settings.Auths))
for _, auth := range settings.Auths {
switch auth.Method {
case MethodNone:
authNameToChecker[auth.Name] = newNoneMethod()
default:
return nil, fmt.Errorf("%w: %s", ErrMethodNotSupported, auth.Name)
}
}

routeToRoles = make(map[string][]internalRole)
for _, role := range settings.Roles {
for _, authName := range role.Auths {
checker, ok := authNameToChecker[authName]
if !ok {
return nil, fmt.Errorf("%w: %s is not one of %s", ErrAuthNameNotDefined,
authName, orStrings(maps.Keys(authNameToChecker)))
}
var checker authorizationChecker
switch role.Auth {
case AuthNone:
checker = newNoneMethod()
default:
return nil, fmt.Errorf("%w: %s", ErrMethodNotSupported, role.Auth)
}

iRole := internalRole{
name: role.Name,
checker: checker,
}
for _, route := range role.Routes {
checkerExists := false
for _, role := range routeToRoles[route] {
if role.checker.equal(iRole.checker) {
checkerExists = true
break
}
}
if checkerExists {
// even if the role name is different, if the checker is the same, skip it.
continue
iRole := internalRole{
name: role.Name,
checker: checker,
}
for _, route := range role.Routes {
checkerExists := false
for _, role := range routeToRoles[route] {
if role.checker.equal(iRole.checker) {
checkerExists = true
break
}
routeToRoles[route] = append(routeToRoles[route], iRole)
}
if checkerExists {
// even if the role name is different, if the checker is the same, skip it.
continue
}
routeToRoles[route] = append(routeToRoles[route], iRole)
}
}
return routeToRoles, nil
Expand Down
18 changes: 3 additions & 15 deletions internal/server/middlewares/auth/lookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,16 @@ func Test_settingsToLookupMap(t *testing.T) {
},
"auth_method_not_supported": {
settings: Settings{
Auths: []Auth{{Name: "bad", Method: "not_supported"}},
Roles: []Role{{Name: "a", Auth: "bad"}},
},
errWrapped: ErrMethodNotSupported,
errMessage: "authentication method not supported: bad",
},
"auth_name_not_defined": {
settings: Settings{
Auths: []Auth{{Name: "x", Method: MethodNone}, {Name: "y", Method: MethodNone}},
Roles: []Role{{Name: "a", Auths: []string{"xyz"}}},
},
errWrapped: ErrAuthNameNotDefined,
errMessage: "authentication name not defined: xyz is not one of x or y",
},
"success": {
settings: Settings{
Auths: []Auth{
{Name: "x", Method: MethodNone},
{Name: "y", Method: MethodNone},
},
Roles: []Role{
{Name: "a", Auths: []string{"x"}, Routes: []string{"GET /path"}},
{Name: "b", Auths: []string{"x", "y"}, Routes: []string{"GET /path", "PUT /path"}},
{Name: "a", Auth: AuthNone, Routes: []string{"GET /path"}},
{Name: "b", Auth: AuthNone, Routes: []string{"GET /path", "PUT /path"}},
},
},
routeToRoles: map[string][]internalRole{
Expand Down
18 changes: 3 additions & 15 deletions internal/server/middlewares/auth/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,8 @@ func Test_authHandler_ServeHTTP(t *testing.T) {
}{
"route_has_no_role": {
settings: Settings{
Auths: []Auth{
{Name: "auth1", Method: MethodNone},
{Name: "auth2", Method: MethodNone},
},
Roles: []Role{
{Name: "role1", Auths: []string{"auth1"}, Routes: []string{"GET /a"}},
{Name: "role1", Auth: AuthNone, Routes: []string{"GET /a"}},
},
},
makeLogger: func(ctrl *gomock.Controller) *MockDebugLogger {
Expand All @@ -46,12 +42,8 @@ func Test_authHandler_ServeHTTP(t *testing.T) {
},
"authorized_unprotected_by_default": {
settings: Settings{
Auths: []Auth{
{Name: "auth1", Method: MethodNone},
{Name: "auth2", Method: MethodNone},
},
Roles: []Role{
{Name: "public", Auths: []string{"auth1"}, Routes: []string{"GET /v1/vpn/status"}},
{Name: "public", Auth: AuthNone, Routes: []string{"GET /v1/vpn/status"}},
},
},
makeLogger: func(ctrl *gomock.Controller) *MockDebugLogger {
Expand All @@ -71,12 +63,8 @@ func Test_authHandler_ServeHTTP(t *testing.T) {
},
"authorized_none": {
settings: Settings{
Auths: []Auth{
{Name: "auth1", Method: MethodNone},
{Name: "auth2", Method: MethodNone},
},
Roles: []Role{
{Name: "role1", Auths: []string{"auth1"}, Routes: []string{"GET /a"}},
{Name: "role1", Auth: AuthNone, Routes: []string{"GET /a"}},
},
},
makeLogger: func(ctrl *gomock.Controller) *MockDebugLogger {
Expand Down
95 changes: 13 additions & 82 deletions internal/server/middlewares/auth/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,15 @@ import (
)

type Settings struct {
// Auths is a list of authentication methods which can be used
// by each role.
Auths []Auth
// Roles is a list of roles with their associated authentication
// and routes.
Roles []Role
}

func (s *Settings) SetDefaults() {
s.Auths = gosettings.DefaultSlice(s.Auths, []Auth{{
Name: "public",
Method: MethodNone,
}}) // TODO v3.41.0 leave empty
s.Roles = gosettings.DefaultSlice(s.Roles, []Role{{ // TODO v3.41.0 leave empty
Name: "public",
Auths: []string{"public"},
Name: "public",
Auth: "none",
Routes: []string{
http.MethodGet + " /openvpn/actions/restart",
http.MethodGet + " /unbound/actions/restart",
Expand All @@ -46,36 +39,8 @@ func (s *Settings) SetDefaults() {
}})
}

var (
ErrAuthNameNotDefined = errors.New("authentication name not defined")
ErrAuthNameNotUnique = errors.New("authentication name is not unique")
)

func (s Settings) Validate() (err error) {
authNameToAuthIndex := make(map[string]int, len(s.Auths))
for i, auth := range s.Auths {
existingIndex, exists := authNameToAuthIndex[auth.Name]
if exists {
return fmt.Errorf("%w: %q for auths %d of %d and %d of %d",
ErrAuthNameNotUnique, auth.Name,
i+1, len(s.Auths), existingIndex+1, len(s.Auths))
}
authNameToAuthIndex[auth.Name] = i

err = auth.validate()
if err != nil {
return fmt.Errorf("auth %d of %d: %w", i+1, len(s.Auths), err)
}
}

for i, role := range s.Roles {
for _, auth := range role.Auths {
_, isDefined := authNameToAuthIndex[auth]
if !isDefined {
return fmt.Errorf("%w: %q for role %s (%d of %d)",
ErrAuthNameNotDefined, auth, role.Name, i+1, len(s.Roles))
}
}
err = role.validate()
if err != nil {
return fmt.Errorf("role %s (%d of %d): %w",
Expand All @@ -87,8 +52,6 @@ func (s Settings) Validate() (err error) {
}

func (s Settings) Copy() (copied Settings) {
copied.Auths = make([]Auth, len(s.Auths))
copy(copied.Auths, s.Auths)
copied.Roles = make([]Role, len(s.Roles))
for i := range s.Roles {
copied.Roles[i] = s.Roles[i].copy()
Expand All @@ -97,19 +60,12 @@ func (s Settings) Copy() (copied Settings) {
}

func (s *Settings) OverrideWith(other Settings) {
s.Auths = gosettings.OverrideWithSlice(s.Auths, other.Auths)
s.Roles = gosettings.OverrideWithSlice(s.Roles, other.Roles)
}

func (s Settings) ToLinesNode() (node *gotree.Node) {
node = gotree.New("Authentication middleware settings:")

authNames := make([]string, len(s.Auths))
for i, auth := range s.Auths {
authNames[i] = auth.Name
}
node.Appendf("Authentications defined: %s", andStrings(authNames))

roleNames := make([]string, len(s.Roles))
for i, role := range s.Roles {
roleNames[i] = role.Name
Expand All @@ -120,57 +76,33 @@ func (s Settings) ToLinesNode() (node *gotree.Node) {
}

const (
MethodNone = "none"
AuthNone = "none"
)

// Auth contains the authentication method name and fields
// specific to each authentication method.
type Auth struct {
// Name is the unique authentication name.
Name string
// Method is the authentication method to use.
Method string
}

func (a Auth) validate() (err error) {
err = validateAuthMethod(a.Method)
if err != nil {
return fmt.Errorf("method for name %s: %w", a.Name, err)
}
return nil
}

var (
ErrMethodNotSupported = errors.New("authentication method not supported")
)

func validateAuthMethod(method string) (err error) {
err = validate.IsOneOf(method, MethodNone)
if err != nil {
return fmt.Errorf("%w: %s", ErrMethodNotSupported, method)
}
return nil
}

// Role contains the role name, authentication method name and
// routes that the role can access.
type Role struct {
// Name is the role name and is only used for documentation
// and in the authentication middleware debug logs.
Name string
// Auths is a list of authentication names that the role can use,
// where each must match a defined authentication.
Auths []string
// Auth is the authentication method to use, which can be 'none'.
Auth string
// Routes is a list of routes that the role can access in the format
// "HTTP_METHOD PATH", for example "GET /v1/vpn/status"
Routes []string
}

var (
ErrRouteNotSupported = errors.New("route not supported by the control server")
ErrMethodNotSupported = errors.New("authentication method not supported")
ErrRouteNotSupported = errors.New("route not supported by the control server")
)

func (r Role) validate() (err error) {
err = validate.IsOneOf(r.Auth, AuthNone)
if err != nil {
return fmt.Errorf("%w: %s", ErrMethodNotSupported, r.Auth)
}

for i, route := range r.Routes {
_, ok := validRoutes[route]
if !ok {
Expand Down Expand Up @@ -205,8 +137,7 @@ var validRoutes = map[string]struct{}{ //nolint:gochecknoglobals

func (r Role) copy() (copied Role) {
copied.Name = r.Name
copied.Auths = make([]string, len(r.Auths))
copy(copied.Auths, r.Auths)
copied.Auth = r.Auth
copied.Routes = make([]string, len(r.Routes))
copy(copied.Routes, r.Routes)
return copied
Expand Down

0 comments on commit 47113b6

Please sign in to comment.