Skip to content

Commit

Permalink
feat(server): resource limits on app tokens (#1959)
Browse files Browse the repository at this point in the history
* WIP new mutation arg

* limited resource token creation done

* token resource rule creation validation

* updated authorizeResolver implementation

* introduced resource access rule checks in authorizeResolver everywhere

* more checks added

* updated projects resolvers

* updated stream resolvers

* more checks added

* error page theme resolution fix

* WIP testss

* more tests

* implemented checks in REST auth pipeline

* REST API coverage & tests

* some tests fixed

* test fixess

* added tests

* feat(server): new automation result reporting scope (#1976)

* feat(server): new automation result reporting scope

* tests fix
  • Loading branch information
fabis94 authored Jan 19, 2024
1 parent 86b535d commit 37d5107
Show file tree
Hide file tree
Showing 54 changed files with 1,678 additions and 312 deletions.
7 changes: 7 additions & 0 deletions packages/frontend-2/error.vue
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
</template>
<script setup lang="ts">
import type { NuxtError } from '#app'
import { useTheme } from '~/lib/core/composables/theme'
/**
* Any errors thrown while rendering this page will cause Nuxt to revert to the default
Expand All @@ -19,10 +20,16 @@ const props = defineProps<{
error: NuxtError
}>()
const { isDarkTheme } = useTheme()
useHead({
title: computed(() => `${props.error.statusCode} - ${props.error.message}`),
bodyAttrs: {
class: 'simple-scrollbar bg-foundation-page text-foreground'
},
htmlAttrs: {
class: computed(() => (isDarkTheme.value ? `dark` : ``)),
lang: 'en'
}
})
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,10 @@ input AutomationRunStatusUpdateInput {
type AutomationMutations {
functionRunStatusReport(input: AutomationRunStatusUpdateInput!): Boolean!
@hasServerRole(role: SERVER_USER)
# @hasScope(scope: "automation:result") # TODO: Consult about this w/ Dim
create(input: AutomationCreateInput!): Boolean! @hasServerRole(role: SERVER_USER)
# @hasScope(scope: "automation:create") # TODO: Consult about this w/ Dim
@hasScope(scope: "automate:report-results")
create(input: AutomationCreateInput!): Boolean!
@hasServerRole(role: SERVER_USER)
@hasScope(scope: "automate:report-results")
}

extend type Mutation {
Expand Down
26 changes: 25 additions & 1 deletion packages/server/assets/core/typedefs/apitoken.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,36 @@ type ApiToken {
lastUsed: DateTime! #date
}

enum TokenResourceIdentifierType {
project
}

type TokenResourceIdentifier {
id: String!
type: TokenResourceIdentifierType!
}

input TokenResourceIdentifierInput {
id: String!
type: TokenResourceIdentifierType!
}

input ApiTokenCreateInput {
scopes: [String!]!
name: String!
lifespan: BigInt
}

input AppTokenCreateInput {
scopes: [String!]!
name: String!
lifespan: BigInt
"""
Optionally limit the token to only have access to specific resources
"""
limitResources: [TokenResourceIdentifierInput!]
}

extend type Mutation {
"""
Creates an personal api token.
Expand All @@ -48,7 +72,7 @@ extend type Mutation {
"""
Create an app token. Only apps can create app tokens and they don't show up under personal access tokens.
"""
appTokenCreate(token: ApiTokenCreateInput!): String!
appTokenCreate(token: AppTokenCreateInput!): String!
@hasServerRole(role: SERVER_USER)
@hasScope(scope: "tokens:write")
}
9 changes: 0 additions & 9 deletions packages/server/assets/core/typedefs/test.graphql

This file was deleted.

14 changes: 9 additions & 5 deletions packages/server/modules/accessrequests/graph/resolvers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { LogicError } from '@/modules/shared/errors'
const resolvers: Resolvers = {
Mutation: {
async streamAccessRequestUse(_parent, args, ctx) {
const { userId } = ctx
const { userId, resourceAccessRules } = ctx
const { requestId, accept, role } = args

if (!userId) throw new LogicError('User ID unexpectedly false')
Expand All @@ -22,7 +22,8 @@ const resolvers: Resolvers = {
userId,
requestId,
accept,
mapStreamRoleToValue(role)
mapStreamRoleToValue(role),
resourceAccessRules
)
return true
},
Expand Down Expand Up @@ -67,9 +68,12 @@ const resolvers: Resolvers = {
throw new LogicError('Unable to find request stream')
}

if (!stream.isPublic) {
await validateStreamAccess(ctx.userId, stream.id, Roles.Stream.Reviewer)
}
await validateStreamAccess(
ctx.userId,
stream.id,
Roles.Stream.Reviewer,
ctx.resourceAccessRules
)

return stream
}
Expand Down
21 changes: 17 additions & 4 deletions packages/server/modules/accessrequests/services/stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ import {
ServerAccessRequestRecord
} from '@/modules/accessrequests/repositories'
import { StreamInvalidAccessError } from '@/modules/core/errors/stream'
import { TokenResourceIdentifier } from '@/modules/core/graph/generated/graphql'
import { Roles, StreamRoles } from '@/modules/core/helpers/mainConstants'
import { getStream } from '@/modules/core/repositories/streams'
import {
addOrUpdateStreamCollaborator,
validateStreamAccess
} from '@/modules/core/services/streams/streamAccessService'
import { ensureError } from '@/modules/shared/helpers/errorHelper'
import { Nullable } from '@/modules/shared/helpers/typeHelper'
import { MaybeNullOrUndefined, Nullable } from '@/modules/shared/helpers/typeHelper'

function buildStreamAccessRequestGraphQLReturn(
record: ServerAccessRequestRecord<AccessRequestType.Stream, string>
Expand Down Expand Up @@ -107,15 +108,21 @@ export async function processPendingStreamRequest(
userId: string,
requestId: string,
accept: boolean,
role: StreamRoles = Roles.Stream.Contributor
role: StreamRoles = Roles.Stream.Contributor,
resourceAccessRules: MaybeNullOrUndefined<TokenResourceIdentifier[]>
) {
const req = await getPendingAccessRequest(requestId, AccessRequestType.Stream)
if (!req) {
throw new AccessRequestProcessingError('No request with this ID exists')
}

try {
await validateStreamAccess(userId, req.resourceId, Roles.Stream.Owner)
await validateStreamAccess(
userId,
req.resourceId,
Roles.Stream.Owner,
resourceAccessRules
)
} catch (e: unknown) {
const err = ensureError(e, 'Stream access validation failed')
if (err instanceof StreamInvalidAccessError) {
Expand All @@ -129,7 +136,13 @@ export async function processPendingStreamRequest(
}

if (accept) {
await addOrUpdateStreamCollaborator(req.resourceId, req.requesterId, role, userId)
await addOrUpdateStreamCollaborator(
req.resourceId,
req.requesterId,
role,
userId,
resourceAccessRules
)
}

await deleteRequestById(req.id)
Expand Down
8 changes: 4 additions & 4 deletions packages/server/modules/activitystream/tests/activity.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,27 +246,27 @@ describe('Activity @activity', () => {
const res = await sendRequest(userIz.token, {
query: `query {stream(id:"${streamSecret.id}") {name activity {items {id streamId resourceType resourceId actionType userId message time}}} }`
})
expect(res.body.errors.length).to.equal(1)
expect(res.body.errors?.length).to.equal(1)
})

it("Should *not* get a stream's activity if you are not a server user", async () => {
const res = await sendRequest(null, {
query: `query {stream(id:"${streamPublic.id}") {name activity {items {id streamId resourceType resourceId actionType userId message time}}} }`
})
expect(res.body.errors.length).to.equal(1)
expect(res.body.errors?.length).to.equal(1)
})

it("Should *not* get a user's activity without the `users:read` scope", async () => {
const res = await sendRequest(userX.token, {
query: `query {otherUser(id:"${userCr.id}") { name activity {items {id streamId resourceType resourceId actionType userId message time}}} }`
})
expect(res.body.errors.length).to.equal(1)
expect(res.body.errors?.length).to.equal(1)
})

it("Should *not* get a user's timeline without the `users:read` scope", async () => {
const res = await sendRequest(userX.token, {
query: `query {otherUser(id:"${userCr.id}") { name timeline {items {id streamId resourceType resourceId actionType userId message time}}} }`
})
expect(res.body.errors.length).to.equal(1)
expect(res.body.errors?.length).to.equal(1)
})
})
3 changes: 2 additions & 1 deletion packages/server/modules/auth/defaultApps.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ const SpeckleAutomate = {
ScopesConst.Users.Read,
ScopesConst.Tokens.Write,
ScopesConst.Streams.Read,
ScopesConst.Streams.Write
ScopesConst.Streams.Write,
ScopesConst.Automate.ReportResults
]
}
20 changes: 19 additions & 1 deletion packages/server/modules/automations/index.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,27 @@
import { moduleLogger } from '@/logging/logging'
import { ScopeRecord } from '@/modules/auth/helpers/types'
import { registerOrUpdateScope } from '@/modules/shared'
import { SpeckleModule } from '@/modules/shared/helpers/typeHelper'
import { Scopes } from '@speckle/shared'

async function initScopes() {
const scopes: ScopeRecord[] = [
{
name: Scopes.Automate.ReportResults,
description: 'Allows the app to report automation results to the server.',
public: false
}
]

for (const scope of scopes) {
await registerOrUpdateScope(scope)
}
}

const automationModule: SpeckleModule = {
init() {
async init() {
moduleLogger.info('🤖 Init automations module')
await initScopes()
}
}

Expand Down
6 changes: 6 additions & 0 deletions packages/server/modules/core/dbSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -519,4 +519,10 @@ export const ServerApps = buildTableHelper('server_apps', [

export const Scopes = buildTableHelper('scopes', ['name', 'description', 'public'])

export const TokenResourceAccess = buildTableHelper('token_resource_access', [
'tokenId',
'resourceType',
'resourceId'
])

export { knex }
7 changes: 6 additions & 1 deletion packages/server/modules/core/graph/directives/hasRole.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,12 @@ module.exports = {
)
}

await authorizeResolver(context.userId, parent.id, requiredRole)
await authorizeResolver(
context.userId,
parent.id,
requiredRole,
context.resourceAccessRules
)
}

const data = await resolve.apply(this, args)
Expand Down
Loading

0 comments on commit 37d5107

Please sign in to comment.