-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Associate role with appkeys #1198
Conversation
03ce7a9
to
699b32d
Compare
9512f3e
to
ee9fd3d
Compare
b834c8c
to
8118b35
Compare
d500e21
to
f45639c
Compare
server/config/options.go
Outdated
@@ -101,6 +101,13 @@ type AuthConfig struct { | |||
EnableErrorLog bool `mapstructure:"enable_error_log" yaml:"enable_error_log" json:"enable_error_log"` | |||
} | |||
|
|||
type ApiKeysConfig struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not related to this change but manual merge has moved it around.
// role names. | ||
readOnlyRoleName = "ro" | ||
editorRoleName = "e" | ||
ownerRoleName = "o" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved it to auth package to avoid circular dependencies.
@@ -450,6 +447,19 @@ func authorize(ctx context.Context) (err error) { | |||
return nil | |||
} | |||
|
|||
/* | |||
func isAuthorizedProject(reqMetadata *request.Metadata, accessToken *types.AccessToken) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keeping it here to enable back the project mismatch authz.
@@ -115,8 +122,15 @@ type GetUserResp struct { | |||
AppMetaData *UserAppData `json:"app_metadata" db:"app_metadata"` | |||
} | |||
|
|||
// returns currentSub, creationTime, error. | |||
func _createAppKey(ctx context.Context, clientId string, clientSecret string, g *gotrue, keyName string, keyDescription string, project string, keyType string) (string, int64, error) { | |||
type keyInfo struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
internal type to avoid long number of args.
func _getEmail(clientId string, appKeyType string, g *gotrue) string { | ||
suffix := g.AuthConfig.Gotrue.UsernameSuffix | ||
if appKeyType == AppKeyTypeApiKey { | ||
suffix = g.AuthConfig.ApiKeys.EmailSuffix | ||
} | ||
return fmt.Sprintf("%s%s", clientId, suffix) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
manual merge moved this method around.
test/v1/server/auth_test.go
Outdated
@@ -193,48 +193,127 @@ func TestGoTrueAuthProvider(t *testing.T) { | |||
Boolean() | |||
require.True(t, deletedResponse.Raw()) | |||
} | |||
func TestApiKeyCrud(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
manual merge moved it around.
bae077c
to
f1f3f4f
Compare
feat: Associate role with appkeys fix: Do not allow higher role app key created fix: Fixed test to follow project level authz fix: rebase fix: rebase fix: rebase fix: Fix the merge fix: User cannot invite user with higher role
f1f3f4f
to
21c2f5a
Compare
🎉 This PR is included in version 1.0.0-beta.117 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Describe your changes
Note: This should not be merged prior to updating interfaces (cloud console, CLI) that accepts the role for app key creation.
How best to test these changes
Added test.
Issue ticket number and link