Skip to content
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: include new validation on validateAdminUserAccess #181

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
4 changes: 3 additions & 1 deletion graphql/directives.graphql
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
directive @withSession on FIELD | FIELD_DEFINITION
directive @withPermissions on FIELD | FIELD_DEFINITION
directive @validateAdminUserAccess on FIELD | FIELD_DEFINITION
directive @validateAdminUserAccess(
orgPermission: String
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to change orgPermission to something more meaningful... maybe requiredPermission?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree

) on FIELD | FIELD_DEFINITION
directive @validateStoreUserAccess on FIELD | FIELD_DEFINITION
directive @checkUserAccess on FIELD | FIELD_DEFINITION
directive @checkAdminAccess on FIELD | FIELD_DEFINITION
Expand Down
3 changes: 2 additions & 1 deletion graphql/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ type Mutation {
notifyUsers: Boolean
): OrganizationCostCenterResponse
@checkAdminAccess
@cacheControl(scope: PRIVATE)
@validateAdminUserAccess(orgPermission: "buyer_organization_edit")
@cacheControl(scope: PRIVATE, maxAge: 300)
createOrganizationAndCostCentersWithId(
input: NormalizedOrganizationInput!
): MasterDataResponse @checkAdminAccess @cacheControl(scope: PRIVATE)
Expand Down
10 changes: 9 additions & 1 deletion node/clients/IdentityClient.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import type { InstanceOptions, IOContext } from '@vtex/api'
import { JanusClient } from '@vtex/api'

type AuthUser = {
id: string
authStatus: string
tokenType: string
account: string
audience: string
user: string
}
export default class IdentityClient extends JanusClient {
constructor(ctx: IOContext, options?: InstanceOptions) {
super(ctx, {
Expand All @@ -11,7 +19,7 @@ export default class IdentityClient extends JanusClient {
})
}

public async validateToken({ token }: { token: string }): Promise<any> {
public async validateToken({ token }: { token: string }): Promise<AuthUser> {
return this.http.post('/api/vtexid/credential/validate', { token })
}

Expand Down
21 changes: 20 additions & 1 deletion node/clients/LMClient.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable no-console */
/* eslint-disable @typescript-eslint/no-explicit-any */
import type { InstanceOptions, IOContext } from '@vtex/api'
import { ExternalClient } from '@vtex/api'
import { ExternalClient, ForbiddenError } from '@vtex/api'

export default class LMClient extends ExternalClient {
constructor(ctx: IOContext, options?: InstanceOptions) {
Expand All @@ -24,6 +25,24 @@ export default class LMClient extends ExternalClient {
})
}

public checkUserAdminPermission = async (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public checkUserAdminPermission = async (
public checkAdminUserRequiredPermission = async (

account: string,
userEmail: string,
resourceCode: string
) => {
const productCode = '97'
giurigaud marked this conversation as resolved.
Show resolved Hide resolved

const checkOrgPermission = await this.get<boolean>(
`/api/license-manager/pvt/accounts/${account}/products/${productCode}/logins/${userEmail}/resources/${resourceCode}/granted`
)

if (!checkOrgPermission) {
throw new ForbiddenError('Unauthorized Access')
}

return checkOrgPermission
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const checkOrgPermission = await this.get<boolean>(
`/api/license-manager/pvt/accounts/${account}/products/${productCode}/logins/${userEmail}/resources/${resourceCode}/granted`
)
if (!checkOrgPermission) {
throw new ForbiddenError('Unauthorized Access')
}
return checkOrgPermission
const checkRequiredPermission = await this.get<boolean>(
`/api/license-manager/pvt/accounts/${account}/products/${productCode}/logins/${userEmail}/resources/${resourceCode}/granted`
)
if (!checkRequiredPermission) {
throw new ForbiddenError('Unauthorized Access')
}
return checkRequiredPermission

}

public getAccount = async () => {
return this.get<GetAccountResponse>(`/api/license-manager/account`).then(
(res) => {
Expand Down
21 changes: 20 additions & 1 deletion node/resolvers/directives/helper.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
/* eslint-disable no-console */

import { isUserPartOfBuyerOrg } from '../Queries/Users'

export const validateAdminToken = async (
context: Context,
adminUserAuthToken: string
adminUserAuthToken: string,
orgPermission?: 'buyer_organization_edit' | 'buyer_organization_view'
giurigaud marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(lets change orgPermission everywhere to requiredPermission

Suggested change
orgPermission?: 'buyer_organization_edit' | 'buyer_organization_view'
requiredPermission?: 'buyer_organization_edit' | 'buyer_organization_view'

): Promise<{
hasAdminToken: boolean
hasValidAdminToken: boolean
Expand All @@ -16,15 +19,19 @@ export const validateAdminToken = async (
// check if has admin token and if it is valid
const hasAdminToken = !!adminUserAuthToken
let hasValidAdminToken = false
let userEmail = ''
let tokenType = ''
// this is used to check if the token is valid by current standards
let hasCurrentValidAdminToken = false

console.log('VALIDATE ADMIN TOKEN')
if (hasAdminToken) {
try {
const authUser = await identity.validateToken({
token: adminUserAuthToken,
})

console.log({ authUser })
// we set this flag to true if the token is valid by current standards
// in the future we should remove this line
hasCurrentValidAdminToken = true
Expand All @@ -34,6 +41,10 @@ export const validateAdminToken = async (
account,
authUser.id
)

userEmail = authUser.user
tokenType = authUser.tokenType
console.log({ userEmail, tokenType })
}
} catch (err) {
// noop so we leave hasValidAdminToken as false
Expand All @@ -44,6 +55,11 @@ export const validateAdminToken = async (
}
}

console.log({ hasValidAdminToken, orgPermission, tokenType })
if (hasValidAdminToken && orgPermission && tokenType === 'user') {
await lm.checkUserAdminPermission(account, userEmail, orgPermission)
}
giurigaud marked this conversation as resolved.
Show resolved Hide resolved

return { hasAdminToken, hasValidAdminToken, hasCurrentValidAdminToken }
}

Expand Down Expand Up @@ -79,6 +95,9 @@ export const validateApiToken = async (
token,
})

console.log('validateApiToken')
console.log({ authUser })
console.log('end validateApiToken')
// we set this flag to true if the token is valid by current standards
// in the future we should remove this line
hasCurrentValidApiToken = true
giurigaud marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
8 changes: 7 additions & 1 deletion node/resolvers/directives/validateAdminUserAccess.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-console */
import { SchemaDirectiveVisitor } from 'graphql-tools'
import { AuthenticationError, ForbiddenError } from '@vtex/api'
import type { GraphQLField } from 'graphql'
Expand All @@ -15,6 +16,10 @@ export class ValidateAdminUserAccess extends SchemaDirectiveVisitor {
public visitFieldDefinition(field: GraphQLField<any, any>) {
const { resolve = defaultFieldResolver } = field

console.log('Directive arguments:', this.args)
const { orgPermission } = this.args

console.log('orgPermission:', orgPermission)
field.resolve = async (
root: any,
args: any,
Expand Down Expand Up @@ -43,7 +48,8 @@ export class ValidateAdminUserAccess extends SchemaDirectiveVisitor {

const { hasAdminToken, hasValidAdminToken } = await validateAdminToken(
context,
adminUserAuthToken as string
adminUserAuthToken as string,
orgPermission
)

// add admin token metrics
Expand Down