Skip to content

Commit

Permalink
fix(core): Make OAuth1/OAuth2 callback not require auth
Browse files Browse the repository at this point in the history
  • Loading branch information
netroy committed Jul 31, 2024
1 parent cf73e29 commit c08b68d
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 22 deletions.
4 changes: 0 additions & 4 deletions packages/cli/src/auth/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ const skipBrowserIdCheckEndpoints = [

// We need to exclude binary-data downloading endpoint because we can't send custom headers on `<embed>` tags
`/${restEndpoint}/binary-data/`,

// oAuth callback urls aren't called by the frontend. therefore we can't send custom header on these requests
`/${restEndpoint}/oauth1-credential/callback`,
`/${restEndpoint}/oauth2-credential/callback`,
];

@Service()
Expand Down
15 changes: 5 additions & 10 deletions packages/cli/src/controllers/oauth/oAuth1Credential.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,8 @@ export class OAuth1CredentialController extends AbstractOAuthController {
}

/** Verify and store app code. Generate access tokens and store for respective credential */
@Get('/callback', { usesTemplates: true })
@Get('/callback', { usesTemplates: true, skipAuth: true })
async handleCallback(req: OAuthRequest.OAuth1Credential.Callback, res: Response) {
const userId = req.user?.id;
try {
const { oauth_verifier, oauth_token, state: encodedState } = req.query;

Expand All @@ -124,7 +123,7 @@ export class OAuth1CredentialController extends AbstractOAuthController {
const credential = await this.getCredentialWithoutUser(credentialId);
if (!credential) {
const errorMessage = 'OAuth1 callback failed because of insufficient permissions';
this.logger.error(errorMessage, { userId, credentialId });
this.logger.error(errorMessage, { credentialId });
return this.renderCallbackError(res, errorMessage);
}

Expand All @@ -138,7 +137,7 @@ export class OAuth1CredentialController extends AbstractOAuthController {

if (this.verifyCsrfState(decryptedDataOriginal, state)) {
const errorMessage = 'The OAuth1 callback state is invalid!';
this.logger.debug(errorMessage, { userId, credentialId });
this.logger.debug(errorMessage, { credentialId });
return this.renderCallbackError(res, errorMessage);
}

Expand All @@ -156,7 +155,7 @@ export class OAuth1CredentialController extends AbstractOAuthController {
try {
oauthToken = await axios.request(options);
} catch (error) {
this.logger.error('Unable to fetch tokens for OAuth1 callback', { userId, credentialId });
this.logger.error('Unable to fetch tokens for OAuth1 callback', { credentialId });
const errorResponse = new NotFoundError('Unable to get access tokens!');
return sendErrorResponse(res, errorResponse);
}
Expand All @@ -172,15 +171,11 @@ export class OAuth1CredentialController extends AbstractOAuthController {
await this.encryptAndSaveData(credential, decryptedDataOriginal);

this.logger.verbose('OAuth1 callback successful for new credential', {
userId,
credentialId,
});
return res.render('oauth-callback');
} catch (error) {
this.logger.error('OAuth1 callback failed because of insufficient user permissions', {
userId,
});
// Error response
this.logger.error('OAuth1 callback failed because of insufficient user permissions');
return sendErrorResponse(res, error as Error);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,8 @@ export class OAuth2CredentialController extends AbstractOAuthController {
}

/** Verify and store app code. Generate access tokens and store for respective credential */
@Get('/callback', { usesTemplates: true })
@Get('/callback', { usesTemplates: true, skipAuth: true })
async handleCallback(req: OAuthRequest.OAuth2Credential.Callback, res: Response) {
const userId = req.user?.id;
try {
const { code, state: encodedState } = req.query;
if (!code || !encodedState) {
Expand All @@ -104,7 +103,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {
const credential = await this.getCredentialWithoutUser(credentialId);
if (!credential) {
const errorMessage = 'OAuth2 callback failed because of insufficient permissions';
this.logger.error(errorMessage, { userId, credentialId });
this.logger.error(errorMessage, { credentialId });
return this.renderCallbackError(res, errorMessage);
}

Expand All @@ -118,7 +117,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {

if (this.verifyCsrfState(decryptedDataOriginal, state)) {
const errorMessage = 'The OAuth2 callback state is invalid!';
this.logger.debug(errorMessage, { userId, credentialId });
this.logger.debug(errorMessage, { credentialId });
return this.renderCallbackError(res, errorMessage);
}

Expand Down Expand Up @@ -157,7 +156,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {

if (oauthToken === undefined) {
const errorMessage = 'Unable to get OAuth2 access tokens!';
this.logger.error(errorMessage, { userId, credentialId });
this.logger.error(errorMessage, { credentialId });
return this.renderCallbackError(res, errorMessage);
}

Expand All @@ -174,7 +173,6 @@ export class OAuth2CredentialController extends AbstractOAuthController {
await this.encryptAndSaveData(credential, decryptedDataOriginal);

this.logger.verbose('OAuth2 callback successful for credential', {
userId,
credentialId,
});

Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ export declare namespace MFA {
export declare namespace OAuthRequest {
namespace OAuth1Credential {
type Auth = AuthenticatedRequest<{}, {}, {}, { id: string }>;
type Callback = AuthenticatedRequest<
type Callback = AuthlessRequest<
{},
{},
{},
Expand All @@ -383,7 +383,7 @@ export declare namespace OAuthRequest {

namespace OAuth2Credential {
type Auth = AuthenticatedRequest<{}, {}, {}, { id: string }>;
type Callback = AuthenticatedRequest<{}, {}, {}, { code: string; state: string }>;
type Callback = AuthlessRequest<{}, {}, {}, { code: string; state: string }>;
}
}

Expand Down

0 comments on commit c08b68d

Please sign in to comment.