apitech
/
grafana
mirror of https://github.com/grafana/grafana
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Identity: Use typed version of namespace id (#87257)

Browse Source 
* Remove different constructors and only use NewNamespaceID

* AdminUser: check typed namespace id

* Identity: Add convinient function to parse valid user id when type is either user or service account

* Annotations: Use typed namespace id instead
pull/87499/head
Karl Persson 2 years ago committed by GitHub
parent d83cbe4d85
commit be5ced4287
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 52 additions and 74 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 11
      pkg/api/admin_users.go
  2. 15
      pkg/api/annotations.go
  3. 2
      pkg/services/accesscontrol/authorize_in_org_test.go
  4. 34
      pkg/services/auth/identity/namespace.go
  5. 12
      pkg/services/authn/authnimpl/service_test.go
  6. 10
      pkg/services/authn/authnimpl/sync/rbac_sync_test.go
  7. 2
      pkg/services/authn/authnimpl/sync/user_sync.go
  8. 6
      pkg/services/authn/authnimpl/sync/user_sync_test.go
  9. 4
      pkg/services/authn/clients/api_key.go
  10. 2
      pkg/services/authn/clients/grafana.go
  11. 2
      pkg/services/authn/clients/proxy.go
  12. 2
      pkg/services/authn/clients/proxy_test.go
  13. 4
      pkg/services/authn/clients/render.go
  14. 2
      pkg/services/authn/clients/session.go
  15. 4
      pkg/services/authn/namespace.go
  16. 2
      pkg/services/contexthandler/contexthandler_test.go
  17. 2
      pkg/services/user/userimpl/verifier.go

11
pkg/api/admin_users.go
Unescape View File

@ -13,7 +13,7 @@ import (
"github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/auth"
"github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/authn"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/org"
@ -366,16 +366,9 @@ func (hs *HTTPServer) AdminLogoutUser(c *contextmodel.ReqContext) response.Respo
return response.Error(http.StatusBadRequest, "id is invalid", err)
}
namespace, identifier := c.SignedInUser.GetNamespacedID()
if namespace == identity.NamespaceUser {
activeUserID, err := identity.IntIdentifier(namespace, identifier)
if err != nil {
return response.Error(http.StatusInternalServerError, "Failed to parse active user id", err)
}
if activeUserID == userID {
if c.SignedInUser.GetID() == authn.NewNamespaceID(authn.NamespaceUser, userID) {
return response.Error(http.StatusBadRequest, "You cannot logout yourself", nil)
}
}
return hs.logoutUserFromAllDevicesInternal(c.Req.Context(), userID)
}

15
pkg/api/annotations.go
Unescape View File

@ -11,7 +11,6 @@ import (
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/annotations"
"github.com/grafana/grafana/pkg/services/auth/identity"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/featuremgmt"
@ -141,7 +140,7 @@ func (hs *HTTPServer) PostAnnotation(c *contextmodel.ReqContext) response.Respon
return response.Error(http.StatusBadRequest, "Failed to save annotation", err)
}
userID, err := identity.UserIdentifier(c.SignedInUser.GetNamespacedID())
userID, err := c.SignedInUser.GetID().UserID()
if err != nil {
return response.Error(http.StatusInternalServerError, "Failed to save annotation", err)
}
@ -228,7 +227,7 @@ func (hs *HTTPServer) PostGraphiteAnnotation(c *contextmodel.ReqContext) respons
return response.Error(http.StatusBadRequest, "Failed to save Graphite annotation", err)
}
userID, err := identity.UserIdentifier(c.SignedInUser.GetNamespacedID())
userID, err := c.SignedInUser.GetID().UserID()
if err != nil {
return response.Error(http.StatusInternalServerError, "Failed to save Graphite annotation", err)
}
@ -285,10 +284,9 @@ func (hs *HTTPServer) UpdateAnnotation(c *contextmodel.ReqContext) response.Resp
}
}
userID, err := identity.UserIdentifier(c.SignedInUser.GetNamespacedID())
userID, err := c.SignedInUser.GetID().UserID()
if err != nil {
return response.Error(http.StatusInternalServerError,
"Failed to update annotation", err)
return response.Error(http.StatusInternalServerError, "Failed to update annotation", err)
}
item := annotations.Item{
@ -348,10 +346,9 @@ func (hs *HTTPServer) PatchAnnotation(c *contextmodel.ReqContext) response.Respo
}
}
userID, err := identity.UserIdentifier(c.SignedInUser.GetNamespacedID())
userID, err := c.SignedInUser.GetID().UserID()
if err != nil {
return response.Error(http.StatusInternalServerError,
"Failed to update annotation", err)
return response.Error(http.StatusInternalServerError, "Failed to update annotation", err)
}
existing := annotations.Item{

2
pkg/services/accesscontrol/authorize_in_org_test.go
Unescape View File

@ -186,7 +186,7 @@ func TestAuthorizeInOrgMiddleware(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, "/api/endpoint", nil)
expectedIdentity := &authn.Identity{
ID: authn.MustNewNamespaceID(authn.NamespaceUser, tc.ctxSignedInUser.UserID),
ID: authn.NewNamespaceID(authn.NamespaceUser, tc.ctxSignedInUser.UserID),
OrgID: tc.targetOrgId,
Permissions: map[int64]map[string][]string{},
}

34
pkg/services/auth/identity/namespace.go
Unescape View File

@ -41,7 +41,7 @@ func ParseNamespace(str string) (Namespace, error) {
}
}
var AnonymousNamespaceID = MustNewNamespaceID(NamespaceAnonymous, 0)
var AnonymousNamespaceID = NewNamespaceID(NamespaceAnonymous, 0)
func ParseNamespaceID(str string) (NamespaceID, error) {
var namespaceID NamespaceID
@ -72,27 +72,7 @@ func MustParseNamespaceID(str string) NamespaceID {
return namespaceID
}
// NewNamespaceID creates a new NamespaceID, will fail for invalid namespace.
func NewNamespaceID(namespace Namespace, id int64) (NamespaceID, error) {
return NamespaceID{
id: strconv.FormatInt(id, 10),
namespace: namespace,
}, nil
}
// MustNewNamespaceID creates a new NamespaceID, will panic for invalid namespace.
// Suitable to use in tests or when we can guarantee that we pass a correct format.
func MustNewNamespaceID(namespace Namespace, id int64) NamespaceID {
namespaceID, err := NewNamespaceID(namespace, id)
if err != nil {
panic(err)
}
return namespaceID
}
// NewNamespaceIDUnchecked creates a new NamespaceID without checking if namespace is valid.
// It us up to the caller to ensure that namespace is valid.
func NewNamespaceIDUnchecked(namespace Namespace, id int64) NamespaceID {
func NewNamespaceID(namespace Namespace, id int64) NamespaceID {
return NamespaceID{
id: strconv.FormatInt(id, 10),
namespace: namespace,
@ -117,6 +97,16 @@ func (ni NamespaceID) ID() string {
return ni.id
}
// UserID will try to parse and int64 identifier if namespace is either user or service-account.
// For all other namespaces '0' will be returned.
func (ni NamespaceID) UserID() (int64, error) {
if ni.IsNamespace(NamespaceUser, NamespaceServiceAccount) {
return ni.ParseInt()
}
return 0, nil
}
// ParseInt will try to parse the id as an int64 identifier.
func (ni NamespaceID) ParseInt() (int64, error) {
return strconv.ParseInt(ni.id, 10, 64)
}

12
pkg/services/authn/authnimpl/service_test.go
Unescape View File

@ -419,31 +419,31 @@ func TestService_Logout(t *testing.T) {
tests := []TestCase{
{
desc: "should redirect to default redirect url when identity is not a user",
identity: &authn.Identity{ID: authn.MustNewNamespaceID(authn.NamespaceServiceAccount, 1)},
identity: &authn.Identity{ID: authn.NewNamespaceID(authn.NamespaceServiceAccount, 1)},
expectedRedirect: &authn.Redirect{URL: "http://localhost:3000/login"},
},
{
desc: "should redirect to default redirect url when no external provider was used to authenticate",
identity: &authn.Identity{ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1)},
identity: &authn.Identity{ID: authn.NewNamespaceID(authn.NamespaceUser, 1)},
expectedRedirect: &authn.Redirect{URL: "http://localhost:3000/login"},
expectedTokenRevoked: true,
},
{
desc: "should redirect to default redirect url when client is not found",
identity: &authn.Identity{ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), AuthenticatedBy: "notfound"},
identity: &authn.Identity{ID: authn.NewNamespaceID(authn.NamespaceUser, 1), AuthenticatedBy: "notfound"},
expectedRedirect: &authn.Redirect{URL: "http://localhost:3000/login"},
expectedTokenRevoked: true,
},
{
desc: "should redirect to default redirect url when client do not implement logout extension",
identity: &authn.Identity{ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), AuthenticatedBy: "azuread"},
identity: &authn.Identity{ID: authn.NewNamespaceID(authn.NamespaceUser, 1), AuthenticatedBy: "azuread"},
expectedRedirect: &authn.Redirect{URL: "http://localhost:3000/login"},
client: &authntest.FakeClient{ExpectedName: "auth.client.azuread"},
expectedTokenRevoked: true,
},
{
desc: "should redirect to client specific url",
identity: &authn.Identity{ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), AuthenticatedBy: "azuread"},
identity: &authn.Identity{ID: authn.NewNamespaceID(authn.NamespaceUser, 1), AuthenticatedBy: "azuread"},
expectedRedirect: &authn.Redirect{URL: "http://idp.com/logout"},
client: &authntest.MockClient{
NameFunc: func() string { return "auth.client.azuread" },
@ -487,7 +487,7 @@ func TestService_Logout(t *testing.T) {
func TestService_ResolveIdentity(t *testing.T) {
t.Run("should return error for for unknown namespace", func(t *testing.T) {
svc := setupTests(t)
_, err := svc.ResolveIdentity(context.Background(), 1, authn.NewNamespaceIDUnchecked("some", 1))
_, err := svc.ResolveIdentity(context.Background(), 1, authn.NewNamespaceID("some", 1))
assert.ErrorIs(t, err, authn.ErrUnsupportedIdentity)
})

10
pkg/services/authn/authnimpl/sync/rbac_sync_test.go
Unescape View File

@ -64,7 +64,7 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) {
desc: "should call sync when authenticated with grafana com and has viewer role",
module: login.GrafanaComAuthModule,
identity: &authn.Identity{
ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1),
ID: authn.NewNamespaceID(authn.NamespaceUser, 1),
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleViewer},
},
@ -75,7 +75,7 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) {
desc: "should call sync when authenticated with grafana com and has editor role",
module: login.GrafanaComAuthModule,
identity: &authn.Identity{
ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1),
ID: authn.NewNamespaceID(authn.NamespaceUser, 1),
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleEditor},
},
@ -86,7 +86,7 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) {
desc: "should call sync when authenticated with grafana com and has admin role",
module: login.GrafanaComAuthModule,
identity: &authn.Identity{
ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1),
ID: authn.NewNamespaceID(authn.NamespaceUser, 1),
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin},
},
@ -97,7 +97,7 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) {
desc: "should not call sync when authenticated with grafana com and has invalid role",
module: login.GrafanaComAuthModule,
identity: &authn.Identity{
ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1),
ID: authn.NewNamespaceID(authn.NamespaceUser, 1),
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleType("something else")},
},
@ -108,7 +108,7 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) {
desc: "should not call sync when not authenticated with grafana com",
module: login.LDAPAuthModule,
identity: &authn.Identity{
ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1),
ID: authn.NewNamespaceID(authn.NamespaceUser, 1),
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin},
},

2
pkg/services/authn/authnimpl/sync/user_sync.go
Unescape View File

@ -389,7 +389,7 @@ func (s *UserSync) lookupByOneOf(ctx context.Context, params login.UserLookupPar
// syncUserToIdentity syncs a user to an identity.
// This is used to update the identity with the latest user information.
func syncUserToIdentity(usr *user.User, id *authn.Identity) {
id.ID = authn.NewNamespaceIDUnchecked(authn.NamespaceUser, usr.ID)
id.ID = authn.NewNamespaceID(authn.NamespaceUser, usr.ID)
id.Login = usr.Login
id.Email = usr.Email
id.Name = usr.Name

6
pkg/services/authn/authnimpl/sync/user_sync_test.go
Unescape View File

@ -470,7 +470,7 @@ func TestUserSync_EnableDisabledUserHook(t *testing.T) {
{
desc: "should skip if correct flag is not set",
identity: &authn.Identity{
ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1),
ID: authn.NewNamespaceID(authn.NamespaceUser, 1),
IsDisabled: true,
ClientParams: authn.ClientParams{EnableUser: false},
},
@ -479,7 +479,7 @@ func TestUserSync_EnableDisabledUserHook(t *testing.T) {
{
desc: "should skip if identity is not a user",
identity: &authn.Identity{
ID: authn.MustNewNamespaceID(authn.NamespaceAPIKey, 1),
ID: authn.NewNamespaceID(authn.NamespaceAPIKey, 1),
IsDisabled: true,
ClientParams: authn.ClientParams{EnableUser: true},
},
@ -488,7 +488,7 @@ func TestUserSync_EnableDisabledUserHook(t *testing.T) {
{
desc: "should enabled disabled user",
identity: &authn.Identity{
ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1),
ID: authn.NewNamespaceID(authn.NamespaceUser, 1),
IsDisabled: true,
ClientParams: authn.ClientParams{EnableUser: true},
},

4
pkg/services/authn/clients/api_key.go
Unescape View File

@ -255,7 +255,7 @@ func validateApiKey(orgID int64, key *apikey.APIKey) error {
func newAPIKeyIdentity(key *apikey.APIKey) *authn.Identity {
return &authn.Identity{
ID: authn.NewNamespaceIDUnchecked(authn.NamespaceAPIKey, key.ID),
ID: authn.NewNamespaceID(authn.NamespaceAPIKey, key.ID),
OrgID: key.OrgID,
OrgRoles: map[int64]org.RoleType{key.OrgID: key.Role},
ClientParams: authn.ClientParams{SyncPermissions: true},
@ -265,7 +265,7 @@ func newAPIKeyIdentity(key *apikey.APIKey) *authn.Identity {
func newServiceAccountIdentity(key *apikey.APIKey) *authn.Identity {
return &authn.Identity{
ID: authn.NewNamespaceIDUnchecked(authn.NamespaceServiceAccount, *key.ServiceAccountId),
ID: authn.NewNamespaceID(authn.NamespaceServiceAccount, *key.ServiceAccountId),
OrgID: key.OrgID,
AuthenticatedBy: login.APIKeyAuthModule,
ClientParams: authn.ClientParams{FetchSyncedUser: true, SyncPermissions: true},

2
pkg/services/authn/clients/grafana.go
Unescape View File

@ -105,7 +105,7 @@ func (c *Grafana) AuthenticatePassword(ctx context.Context, r *authn.Request, us
}
return &authn.Identity{
ID: authn.NewNamespaceIDUnchecked(authn.NamespaceUser, usr.ID),
ID: authn.NewNamespaceID(authn.NamespaceUser, usr.ID),
OrgID: r.OrgID,
ClientParams: authn.ClientParams{FetchSyncedUser: true, SyncPermissions: true},
AuthenticatedBy: login.PasswordAuthModule,

2
pkg/services/authn/clients/proxy.go
Unescape View File

@ -124,7 +124,7 @@ func (c *Proxy) retrieveIDFromCache(ctx context.Context, cacheKey string, r *aut
}
return &authn.Identity{
ID: authn.NewNamespaceIDUnchecked(authn.NamespaceUser, uid),
ID: authn.NewNamespaceID(authn.NamespaceUser, uid),
OrgID: r.OrgID,
// FIXME: This does not match the actual auth module used, but should not have any impact
// Maybe caching the auth module used with the user ID would be a good idea

2
pkg/services/authn/clients/proxy_test.go
Unescape View File

@ -203,7 +203,7 @@ func TestProxy_Hook(t *testing.T) {
}
cache := &fakeCache{data: make(map[string][]byte)}
userId := int64(1)
userID := authn.MustNewNamespaceID(authn.NamespaceUser, userId)
userID := authn.NewNamespaceID(authn.NamespaceUser, userId)
// withRole creates a test case for a user with a specific role.
withRole := func(role string) func(t *testing.T) {

4
pkg/services/authn/clients/render.go
Unescape View File

@ -42,7 +42,7 @@ func (c *Render) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide
if renderUsr.UserID <= 0 {
return &authn.Identity{
ID: authn.NewNamespaceIDUnchecked(authn.NamespaceRenderService, 0),
ID: authn.NewNamespaceID(authn.NamespaceRenderService, 0),
OrgID: renderUsr.OrgID,
OrgRoles: map[int64]org.RoleType{renderUsr.OrgID: org.RoleType(renderUsr.OrgRole)},
ClientParams: authn.ClientParams{SyncPermissions: true},
@ -52,7 +52,7 @@ func (c *Render) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide
}
return &authn.Identity{
ID: authn.NewNamespaceIDUnchecked(authn.NamespaceUser, renderUsr.UserID),
ID: authn.NewNamespaceID(authn.NamespaceUser, renderUsr.UserID),
LastSeenAt: time.Now(),
AuthenticatedBy: login.RenderModule,
ClientParams: authn.ClientParams{FetchSyncedUser: true, SyncPermissions: true},

2
pkg/services/authn/clients/session.go
Unescape View File

@ -57,7 +57,7 @@ func (s *Session) Authenticate(ctx context.Context, r *authn.Request) (*authn.Id
}
ident := &authn.Identity{
ID: authn.NewNamespaceIDUnchecked(authn.NamespaceUser, token.UserId),
ID: authn.NewNamespaceID(authn.NamespaceUser, token.UserId),
SessionToken: token,
ClientParams: authn.ClientParams{
FetchSyncedUser: true,

4
pkg/services/authn/namespace.go
Unescape View File

@ -13,7 +13,7 @@ const (
NamespaceAccessPolicy = identity.NamespaceAccessPolicy
)
var AnonymousNamespaceID = MustNewNamespaceID(NamespaceAnonymous, 0)
var AnonymousNamespaceID = NewNamespaceID(NamespaceAnonymous, 0)
type NamespaceID = identity.NamespaceID
@ -21,7 +21,5 @@ var (
ParseNamespaceID = identity.ParseNamespaceID
MustParseNamespaceID = identity.MustParseNamespaceID
NewNamespaceID = identity.NewNamespaceID
MustNewNamespaceID = identity.MustNewNamespaceID
NewNamespaceIDUnchecked = identity.NewNamespaceIDUnchecked
ErrInvalidNamespaceID = identity.ErrInvalidNamespaceID
)

2
pkg/services/contexthandler/contexthandler_test.go
Unescape View File

@ -44,7 +44,7 @@ func TestContextHandler(t *testing.T) {
})
t.Run("should set identity on successful authentication", func(t *testing.T) {
identity := &authn.Identity{ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), OrgID: 1}
identity := &authn.Identity{ID: authn.NewNamespaceID(authn.NamespaceUser, 1), OrgID: 1}
handler := contexthandler.ProvideService(
setting.NewCfg(),
tracing.InitializeTracerForTest(),

2
pkg/services/user/userimpl/verifier.go
Unescape View File

@ -152,6 +152,6 @@ func (s *Verifier) Complete(ctx context.Context, cmd user.CompleteEmailVerifyCom
// remove the current token, so a new one can be generated with correct values.
return s.is.RemoveIDToken(
ctx,
&authn.Identity{ID: authn.NewNamespaceIDUnchecked(authn.NamespaceUser, usr.ID), OrgID: usr.OrgID},
&authn.Identity{ID: authn.NewNamespaceID(authn.NamespaceUser, usr.ID), OrgID: usr.OrgID},
)
}

Write Preview
Loading…
Cancel
Save