Skip to content

Commit

Permalink
Use refactored external auth config for managed robots (#2046)
Browse files Browse the repository at this point in the history 
Co-authored-by: Michael Lee <[email protected]>
  • Loading branch information
@edaniels @michaellee1019
edaniels and michaellee1019 authored Mar 20, 2023
1 parent db58837 commit 28b64c8
Show file tree
Hide file tree
Showing 15 changed files with 165 additions and 213 deletions.
4 changes: 2 additions & 2 deletions cli/auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ const (

prodAuthDomain = "https://auth.viam.com"
prodAudience = "https://app.viam.com/"
prodClientID = "HysEkkRKn6cDr2W6UFI6UYJHpiVwXFCk" // cli client
prodClientID = "HysEkkRKn6cDr2W6UFI6UYJHpiVwXFCk" // native client ID

stgAuthDomain = "https://auth.viam.dev"
stgAudience = "https://app.viam.dev/"
stgClientID = "o75PSAO21337n6SE0IV2BF9Aj9Er9NF6" // cli client
stgClientID = "o75PSAO21337n6SE0IV2BF9Aj9Er9NF6" // native client ID

defaultWaitInterval = time.Second * 1

Expand Down
2 changes: 1 addition & 1 deletion cli/viam/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func main() {
},
Before: func(c *cli.Context) error {
if c.Bool("debug") {
logger = golog.NewDevelopmentLogger("cli")
logger = golog.NewDebugLogger("cli")
} else {
logger = zap.NewNop().Sugar()
}
Expand Down
47 changes: 20 additions & 27 deletions config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"go.viam.com/utils/jwks"
"go.viam.com/utils/pexec"
"go.viam.com/utils/rpc"
"go.viam.com/utils/rpc/oauth"

"go.viam.com/rdk/referenceframe"
rutils "go.viam.com/rdk/utils"
Expand Down Expand Up @@ -569,12 +568,12 @@ func (sc *SessionsConfig) Validate(path string) error {
type AuthConfig struct {
Handlers []AuthHandlerConfig `json:"handlers"`
TLSAuthEntities []string `json:"tls_auth_entities"`
// TODO(erd): test
ExternalAuthConfig *ExternalAuthConfig `json:"external_auth_config,omitempty"`
}

// WebOAuthConfig contains the structued AuthHandlerConfig for the CredentialsTypeOAuthWeb type.
type WebOAuthConfig struct {
// The allowed jwt audiences the OAuthWeb auth handler will accept.
AllowedAudiences []string `json:"allowed_audiences"`
// ExternalAuthConfig contains information needed to verify externally authenticated tokens.
type ExternalAuthConfig struct {
// contains the raw jwks json.
JSONKeySet AttributeMap `json:"jwks"`

Expand All @@ -583,19 +582,19 @@ type WebOAuthConfig struct {
}

var (
allowedKeyTypesForWebOAuth = map[string]bool{
allowedKeyTypesForExternalAuth = map[string]bool{
"RSA": true,
}

allowedAlgsForWebOAuth = map[string]bool{
allowedAlgsForExternalAuth = map[string]bool{
"RS256": true,
"RS384": true,
"RS512": true,
}
)

// Validate returns true if the WebOAuth is valid. Ensures each key is valid and meets the required constraints.
func (c *WebOAuthConfig) Validate(path string) error {
// Validate returns true if the config is valid. Ensures each key is valid and meets the required constraints.
func (c *ExternalAuthConfig) Validate(path string) error {
jwksPath := fmt.Sprintf("%s.jwks", path)
jsonJWKs, err := json.Marshal(c.JSONKeySet)
if err != nil {
Expand All @@ -618,12 +617,12 @@ func (c *WebOAuthConfig) Validate(path string) error {
return utils.NewConfigValidationError(fmt.Sprintf("%s.%d", jwksPath, i), errors.New("failed to parse jwks, missing index"))
}

if _, ok := allowedKeyTypesForWebOAuth[key.KeyType().String()]; !ok {
if _, ok := allowedKeyTypesForExternalAuth[key.KeyType().String()]; !ok {
return utils.NewConfigValidationError(fmt.Sprintf("%s.%d", jwksPath, i),
errors.Errorf("failed to parse jwks, invalid key type (%s) only (RSA) supported", key.KeyType().String()))
}

if _, ok := allowedAlgsForWebOAuth[key.Algorithm()]; !ok {
if _, ok := allowedAlgsForExternalAuth[key.Algorithm()]; !ok {
return utils.NewConfigValidationError(fmt.Sprintf("%s.%d", jwksPath, i),
errors.Errorf("failed to parse jwks, invalid alg (%s) type only (RS256, RS384, RS512) supported", key.Algorithm()))
}
Expand All @@ -638,9 +637,6 @@ func (c *WebOAuthConfig) Validate(path string) error {
type AuthHandlerConfig struct {
Type rpc.CredentialsType `json:"type"`
Config AttributeMap `json:"config"`

// Structured config for credential type CredentialsTypeOAuthWeb. When this is set the Config field should be empty.
WebOAuthConfig *WebOAuthConfig `json:"web_oauth_config,omitempty"`
}

// Validate ensures all parts of the config are valid.
Expand All @@ -656,6 +652,11 @@ func (config *AuthConfig) Validate(path string) error {
return err
}
}
if config.ExternalAuthConfig != nil {
if err := config.ExternalAuthConfig.Validate(fmt.Sprintf("%s.%s", path, "external_auth_config")); err != nil {
return err
}
}
return nil
}

Expand All @@ -669,19 +670,11 @@ func (config *AuthHandlerConfig) Validate(path string) error {
if config.Config.String("key") == "" && len(config.Config.StringSlice("keys")) == 0 {
return utils.NewConfigValidationError(fmt.Sprintf("%s.config", path), errors.New("key or keys is required"))
}
case oauth.CredentialsTypeOAuthWeb:
if len(config.Config) != 0 {
return utils.NewConfigValidationError(fmt.Sprintf("%s.config", path),
errors.Errorf("config should be empty (use web_oauth_config) for type: %q", oauth.CredentialsTypeOAuthWeb))
}

subPath := fmt.Sprintf("%s.%s", path, "web_oauth_config")
if config.WebOAuthConfig == nil {
return utils.NewConfigValidationError(subPath,
errors.Errorf("web_oauth_config is required for type: %q", oauth.CredentialsTypeOAuthWeb))
}

return config.WebOAuthConfig.Validate(subPath)
case rpc.CredentialsTypeExternal:
return errors.New("robot cannot issue external auth tokens")
case rpc.CredentialsType("oauth-web-auth"):
// TODO(APP-1412): remove after a week from being deployed
return nil
default:
return utils.NewConfigValidationError(path, errors.Errorf("do not know how to handle auth for %q", config.Type))
}
Expand Down
86 changes: 12 additions & 74 deletions config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"go.viam.com/utils/jwks"
"go.viam.com/utils/pexec"
"go.viam.com/utils/rpc"
"go.viam.com/utils/rpc/oauth"

"go.viam.com/rdk/components/board"
fakeboard "go.viam.com/rdk/components/board/fake"
Expand Down Expand Up @@ -813,58 +812,18 @@ func TestAuthConfigEnsure(t *testing.T) {
test.That(t, err, test.ShouldBeNil)
})

t.Run("web-oauth handler with config specified", func(t *testing.T) {
t.Run("external auth with invalid keyset", func(t *testing.T) {
config := config.Config{
Auth: config.AuthConfig{
Handlers: []config.AuthHandlerConfig{
{
Type: oauth.CredentialsTypeOAuthWeb,
Config: config.AttributeMap{"key": "abc123"},
},
},
},
}

err := config.Ensure(true)
test.That(t, err.Error(), test.ShouldContainSubstring, "config should be empty (use web_oauth_config)")
})

t.Run("web-oauth handler with missing WebOAuthConfig", func(t *testing.T) {
config := config.Config{
Auth: config.AuthConfig{
Handlers: []config.AuthHandlerConfig{
{
Type: oauth.CredentialsTypeOAuthWeb,
Config: config.AttributeMap{},
},
},
},
}

err := config.Ensure(true)
test.That(t, err.Error(), test.ShouldContainSubstring, "web_oauth_config is required for type")
})

t.Run("web-oauth handler with invalid keyset", func(t *testing.T) {
config := config.Config{
Auth: config.AuthConfig{
Handlers: []config.AuthHandlerConfig{
{
Type: oauth.CredentialsTypeOAuthWeb,
Config: config.AttributeMap{},
WebOAuthConfig: &config.WebOAuthConfig{
AllowedAudiences: []string{"aud1"},
},
},
},
ExternalAuthConfig: &config.ExternalAuthConfig{},
},
}

err := config.Ensure(true)
test.That(t, err.Error(), test.ShouldContainSubstring, "failed to parse jwks")
})

t.Run("web-oauth handler valid config", func(t *testing.T) {
t.Run("external auth valid config", func(t *testing.T) {
algTypes := map[string]bool{
"RS256": true,
"RS384": true,
Expand All @@ -883,24 +842,17 @@ func TestAuthConfigEnsure(t *testing.T) {

config := config.Config{
Auth: config.AuthConfig{
Handlers: []config.AuthHandlerConfig{
{
Type: oauth.CredentialsTypeOAuthWeb,
Config: config.AttributeMap{},
WebOAuthConfig: &config.WebOAuthConfig{
AllowedAudiences: []string{"aud1"},
JSONKeySet: keysetToAttributeMap(t, keyset),
},
},
ExternalAuthConfig: &config.ExternalAuthConfig{
JSONKeySet: keysetToAttributeMap(t, keyset),
},
},
}

err = config.Ensure(true)
test.That(t, err, test.ShouldBeNil)

test.That(t, config.Auth.Handlers[0].WebOAuthConfig.ValidatedKeySet, test.ShouldNotBeNil)
_, ok := config.Auth.Handlers[0].WebOAuthConfig.ValidatedKeySet.LookupKeyID("key-id-1")
test.That(t, config.Auth.ExternalAuthConfig.ValidatedKeySet, test.ShouldNotBeNil)
_, ok := config.Auth.ExternalAuthConfig.ValidatedKeySet.LookupKeyID("key-id-1")
test.That(t, ok, test.ShouldBeTrue)
}
})
Expand All @@ -924,15 +876,8 @@ func TestAuthConfigEnsure(t *testing.T) {

config := config.Config{
Auth: config.AuthConfig{
Handlers: []config.AuthHandlerConfig{
{
Type: oauth.CredentialsTypeOAuthWeb,
Config: config.AttributeMap{},
WebOAuthConfig: &config.WebOAuthConfig{
AllowedAudiences: []string{"aud1"},
JSONKeySet: keysetToAttributeMap(t, keyset),
},
},
ExternalAuthConfig: &config.ExternalAuthConfig{
JSONKeySet: keysetToAttributeMap(t, keyset),
},
},
}
Expand All @@ -943,18 +888,11 @@ func TestAuthConfigEnsure(t *testing.T) {
}
})

t.Run("web-oauth handler no keys", func(t *testing.T) {
t.Run("external auth no keys", func(t *testing.T) {
config := config.Config{
Auth: config.AuthConfig{
Handlers: []config.AuthHandlerConfig{
{
Type: oauth.CredentialsTypeOAuthWeb,
Config: config.AttributeMap{},
WebOAuthConfig: &config.WebOAuthConfig{
AllowedAudiences: []string{"aud1"},
JSONKeySet: keysetToAttributeMap(t, jwk.NewSet()),
},
},
ExternalAuthConfig: &config.ExternalAuthConfig{
JSONKeySet: keysetToAttributeMap(t, jwk.NewSet()),
},
},
}
Expand Down
58 changes: 23 additions & 35 deletions config/proto_conversions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"go.viam.com/utils/pexec"
"go.viam.com/utils/protoutils"
"go.viam.com/utils/rpc"
"go.viam.com/utils/rpc/oauth"
"google.golang.org/protobuf/types/known/durationpb"

"go.viam.com/rdk/referenceframe"
Expand Down Expand Up @@ -591,6 +590,17 @@ func AuthConfigToProto(auth *AuthConfig) (*pb.AuthConfig, error) {
TlsAuthEntities: auth.TLSAuthEntities,
}

if auth.ExternalAuthConfig != nil {
jwksJSON, err := protoutils.StructToStructPb(auth.ExternalAuthConfig.JSONKeySet)
if err != nil {
return nil, errors.Wrap(err, "failed to convert JSONKeySet")
}

proto.ExternalAuthConfig = &pb.ExternalAuthConfig{
Jwks: &pb.JWKSFile{Json: jwksJSON},
}
}

return &proto, nil
}

Expand All @@ -606,6 +616,12 @@ func AuthConfigFromProto(proto *pb.AuthConfig) (*AuthConfig, error) {
TLSAuthEntities: proto.GetTlsAuthEntities(),
}

if proto.ExternalAuthConfig != nil {
auth.ExternalAuthConfig = &ExternalAuthConfig{
JSONKeySet: proto.ExternalAuthConfig.Jwks.Json.AsMap(),
}
}

return &auth, nil
}

Expand Down Expand Up @@ -674,22 +690,6 @@ func authHandlerConfigToProto(handler AuthHandlerConfig) (*pb.AuthHandlerConfig,
Type: credType,
}

if credType == pb.CredentialsType_CREDENTIALS_TYPE_WEB_OAUTH {
if handler.WebOAuthConfig == nil {
return nil, errors.New("missing WebOAuthConfig field for CREDENTIALS_TYPE_WEB_OAUTH AuthHandler type")
}

jwksJSON, err := protoutils.StructToStructPb(handler.WebOAuthConfig.JSONKeySet)
if err != nil {
return nil, errors.Wrap(err, "failed to convert JSONKeySet")
}

out.WebOauthConfig = &pb.AuthHandlerWebOauthConfig{
AllowedAudiences: handler.WebOAuthConfig.AllowedAudiences,
Jwks: &pb.JWKSFile{Json: jwksJSON},
}
}

return out, nil
}

Expand All @@ -700,23 +700,10 @@ func authHandlerConfigFromProto(proto *pb.AuthHandlerConfig) (AuthHandlerConfig,
return handler, err
}

handler = AuthHandlerConfig{
return AuthHandlerConfig{
Config: proto.GetConfig().AsMap(),
Type: credType,
}

if credType == oauth.CredentialsTypeOAuthWeb {
if proto.WebOauthConfig == nil {
return handler, errors.New("missing WebOAuthConfig field for CredentialsTypeOAuthWeb AuthHandler type")
}

handler.WebOAuthConfig = &WebOAuthConfig{
AllowedAudiences: proto.WebOauthConfig.AllowedAudiences,
JSONKeySet: proto.WebOauthConfig.Jwks.Json.AsMap(),
}
}

return handler, nil
}, nil
}

func credentialsTypeToProto(ct rpc.CredentialsType) (pb.CredentialsType, error) {
Expand All @@ -727,8 +714,8 @@ func credentialsTypeToProto(ct rpc.CredentialsType) (pb.CredentialsType, error)
return pb.CredentialsType_CREDENTIALS_TYPE_ROBOT_SECRET, nil
case rutils.CredentialsTypeRobotLocationSecret:
return pb.CredentialsType_CREDENTIALS_TYPE_ROBOT_LOCATION_SECRET, nil
case oauth.CredentialsTypeOAuthWeb:
return pb.CredentialsType_CREDENTIALS_TYPE_WEB_OAUTH, nil
case rpc.CredentialsTypeExternal:
fallthrough
default:
return pb.CredentialsType_CREDENTIALS_TYPE_UNSPECIFIED, errors.New("unsupported credential type")
}
Expand All @@ -743,7 +730,8 @@ func credentialsTypeFromProto(ct pb.CredentialsType) (rpc.CredentialsType, error
case pb.CredentialsType_CREDENTIALS_TYPE_ROBOT_LOCATION_SECRET:
return rutils.CredentialsTypeRobotLocationSecret, nil
case pb.CredentialsType_CREDENTIALS_TYPE_WEB_OAUTH:
return oauth.CredentialsTypeOAuthWeb, nil
// TODO(APP-1412): remove after a week from being deployed
return rpc.CredentialsType("oauth-web-auth"), nil
case pb.CredentialsType_CREDENTIALS_TYPE_UNSPECIFIED:
fallthrough
case pb.CredentialsType_CREDENTIALS_TYPE_INTERNAL:
Expand Down
Loading

0 comments on commit 28b64c8

Please sign in to comment.