Explicitly catch an error querystring with value access_denied #1333
Conversation
filmaj
added
semver:patch
enhancement
packages/oauth/src/index.spec.js
Outdated
| @@ -417,8 +417,8 @@ describe('OAuth', async () => { | |||
| assert.isTrue(sent); | |||
| }); | |||
|
|
|||
| it('should call the failure callback due to missing code query parameter on the URL', async () => { | |||
There was a problem hiding this comment.
I've essentially removed this test as it is line-for-line exactly the same as the previous test. While the intention of the two tests are different, because the callback handler needs one of a code or a state parameter returned, the result ends up a little off.
Alternatively, we could rewrite these tests a little to check that the callback handler passes if either state or code is provided.
There was a problem hiding this comment.
I've got a branch open with some updates to have these tests properly test state and code and throw different errors in each case. Related to making state optional depending on whether stateValidation flag is true/false.
There was a problem hiding this comment.
Perhaps just add your test separately for now, since it's testing something unrelated to either state or code?
There was a problem hiding this comment.
Sounds good, just pushed it back up!
| let state: string; | ||
| let installOptions: InstallURLOptions; | ||
|
|
||
| try { | ||
| if (req.url !== undefined) { | ||
| parsedUrl = new URL(req.url); | ||
| flowError = parsedUrl.searchParams.get('error') as string; | ||
| if (flowError === 'access_denied') { | ||
| throw new AuthorizationError('User cancelled the OAuth installation flow!'); |
There was a problem hiding this comment.
I decided to reuse an existing error, AuthorizationError, to describe this situation. Let me know if something else is desired.
…signifies user canceled out of the installation flow. This fixes #1186
filmaj
force-pushed
the
oauth-cancel-flow
branch
from
74799c9
to
490f774
Compare
Summary
An
errorquerystring on the oauth redirect URL with a value ofaccess_deniedsignifies that the user canceled out of the installation flow.This PR fixes #1186 / #824
Requirements (place an
xin each[ ])