-
Notifications
You must be signed in to change notification settings - Fork 147
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
Upgrading node-auth0 from v3 to v4 #887
base: master
Are you sure you want to change the base?
Changes from all commits
5014c10
9fd5767
4e37522
437b580
ce80361
da248d0
0c70611
82db2ee
0811df7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -161,19 +161,19 @@ export const setupContext = async ( | |
return new AuthenticationClient({ | ||
domain: AUTH0_DOMAIN, | ||
clientId: AUTH0_CLIENT_ID, | ||
clientAssertionSigningKey: readFileSync(AUTH0_CLIENT_SIGNING_KEY_PATH), | ||
clientAssertionSigningKey: readFileSync(AUTH0_CLIENT_SIGNING_KEY_PATH, 'utf8'), | ||
clientAssertionSigningAlg: !!AUTH0_CLIENT_SIGNING_ALGORITHM | ||
? AUTH0_CLIENT_SIGNING_ALGORITHM | ||
: undefined, | ||
}); | ||
})(); | ||
|
||
const clientCredentials = await authClient.clientCredentialsGrant({ | ||
const clientCredentials = await authClient.oauth.clientCredentialsGrant({ | ||
audience: config.AUTH0_AUDIENCE | ||
? config.AUTH0_AUDIENCE | ||
: `https://${config.AUTH0_DOMAIN}/api/v2/`, | ||
}); | ||
return clientCredentials.access_token; | ||
return clientCredentials.data?.access_token; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should introduce some checks to prevent an undefined |
||
})(); | ||
|
||
const mgmtClient = new ManagementClient({ | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -167,7 +167,7 @@ export default function pagedClient(client: BaseAuth0APIClient): Auth0APIClient | |
frequencyLimit: API_FREQUENCY_PER_SECOND, | ||
frequencyWindow: 1000, // 1 sec | ||
}), | ||
}; | ||
} as unknown as Auth0APIClient; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks very weird. Can the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update: Looking at this with Frederik again, we should probably find a way to exclude the pool property or facilitate pagination some other way that can retain the |
||
|
||
return pagedManager(clientWithPooling, clientWithPooling); | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,4 +1,5 @@ | ||||||
import _ from 'lodash'; | ||||||
import { GetActions200ResponseActionsInner, PostActionRequest } from 'auth0'; | ||||||
import DefaultAPIHandler, { order } from './default'; | ||||||
import log from '../../../logger'; | ||||||
import { areArraysEquals, sleep } from '../../utils'; | ||||||
|
@@ -92,29 +93,34 @@ export function isMarketplaceAction(action: Action): boolean { | |||||
} | ||||||
|
||||||
export default class ActionHandler extends DefaultAPIHandler { | ||||||
existing: Action[] | null; | ||||||
existing: GetActions200ResponseActionsInner[] | null; | ||||||
|
||||||
constructor(options: DefaultAPIHandler) { | ||||||
super({ | ||||||
...options, | ||||||
type: 'actions', | ||||||
functions: { | ||||||
create: (action: Action) => this.createAction(action), | ||||||
create: (action: PostActionRequest) => this.createAction(action), | ||||||
delete: (action: Action) => this.deleteAction(action), | ||||||
}, | ||||||
stripUpdateFields: ['deployed', 'status'], | ||||||
}); | ||||||
} | ||||||
|
||||||
async createAction(action: Action) { | ||||||
async createAction(action: PostActionRequest) { | ||||||
// Strip the deployed flag | ||||||
const addAction = { ...action }; | ||||||
delete addAction.deployed; | ||||||
delete addAction.status; | ||||||
const createdAction = await this.client.actions.create(addAction); | ||||||
|
||||||
// TODO: Should we keep this? | ||||||
delete (addAction as any).deployed; | ||||||
delete (addAction as any).status; | ||||||
Comment on lines
+114
to
+116
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes because when you create an action you cannot specify the |
||||||
|
||||||
const { data: createdAction } = await this.client.actions.create(addAction); | ||||||
// Add the action id so we can deploy it later | ||||||
action.id = createdAction.id; | ||||||
|
||||||
// TODO: Should we keep this? | ||||||
(action as any).id = createdAction.id; | ||||||
Comment on lines
+121
to
+122
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably keep this so we can have reference of the ID. Should double check through testing though. |
||||||
|
||||||
return createdAction; | ||||||
} | ||||||
|
||||||
|
@@ -202,7 +208,9 @@ export default class ActionHandler extends DefaultAPIHandler { | |||||
// Actions API does not support include_totals param like the other paginate API's. | ||||||
// So we set it to false otherwise it will fail with "Additional properties not allowed: include_totals" | ||||||
try { | ||||||
const actions = await this.client.actions.getAll({ paginate: true }); | ||||||
// TODO: bring back paginate: true | ||||||
const { data } = await this.client.actions.getAll(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
const { actions } = data; | ||||||
this.existing = actions; | ||||||
return actions; | ||||||
} catch (err) { | ||||||
|
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.
Yes, this is important to include.
Before Node v4, Auth0 Node SDK didn't natively support pagination and I don't believe that v4 added it either but this needs to be confirmed. ThegetAll
function is mutated by the Deploy CLI to support thepaginate
property.Update: Yes, we need to bring this back and account for the pagination monkey-patch. Perhaps we should extract as a function instead?