Skip to content

Commit

Permalink
fix(authorization): authorization url encoding (#3062)
Browse files Browse the repository at this point in the history
### Describe the problem and your solution
Turns out some of the providers don't like it when we encode some of the
url parts, i.e `scopes`, `base_url` when we are interpolating with
`connectionConfigs`. To help with this, the PR introduces a way we can
handle this within the various parts. Currently we can skip
authorization_url encoding within the `scopes`, or when interpolating
the `base_url` with `connectionConfig`.
<!-- Issue ticket number and link (if applicable) -->

### Testing instructions
When creating a new connection for `Pennylane`, the scopes aren't
encoded `customer_invoices+supplier_invoices+ledger+accounting`

---------

Co-authored-by: Hassan Wari <[email protected]>
  • Loading branch information
hassan254-prog and Hassan Wari authored Nov 27, 2024
1 parent 7160cdf commit 57bac9a
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 12 deletions.
17 changes: 16 additions & 1 deletion packages/server/lib/controllers/oauth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -533,13 +533,28 @@ class OAuthController {
oauth2Client.getSimpleOAuth2ClientConfig(providerConfig, provider, connectionConfig)
);

const scopeSeparator = provider.scope_separator || ' ';
const scopes = providerConfig.oauth_scopes ? providerConfig.oauth_scopes.split(',').join(scopeSeparator) : '';

let authorizationUri = simpleOAuthClient.authorizeURL({
redirect_uri: callbackUrl,
scope: providerConfig.oauth_scopes ? providerConfig.oauth_scopes.split(',').join(provider.scope_separator || ' ') : '',
scope: scopes,
state: session.id,
...allAuthParams
});

if (provider?.authorization_url_skip_encode?.includes('scopes')) {
const url = new URL(authorizationUri);
const queryParams = new URLSearchParams(url.search);
queryParams.delete('scope');
let newQuery = queryParams.toString();
if (scopes) {
newQuery = newQuery ? `${newQuery}&scope=${scopes}` : `scope=${scopes}`;
}
url.search = newQuery;
authorizationUri = url.toString();
}

if (provider.authorization_url_fragment) {
const urlObj = new URL(authorizationUri);
const { search } = urlObj;
Expand Down
8 changes: 4 additions & 4 deletions packages/shared/lib/clients/oauth2.client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ export function getSimpleOAuth2ClientConfig(
connectionConfig: Record<string, string>
): Merge<ModuleOptions, { http: WreckHttpOptions }> {
const templateTokenUrl = typeof provider.token_url === 'string' ? provider.token_url : (provider.token_url!['OAUTH2'] as string);
const tokenUrl = makeUrl(templateTokenUrl, connectionConfig, provider.token_url_encode);
const authorizeUrl = makeUrl(provider.authorization_url!, connectionConfig, provider.authorization_url_encode);
const tokenUrl = makeUrl(templateTokenUrl, connectionConfig, provider.token_url_skip_encode);
const authorizeUrl = makeUrl(provider.authorization_url!, connectionConfig, provider.authorization_url_skip_encode);

const headers = { 'User-Agent': 'Nango' };

Expand Down Expand Up @@ -157,9 +157,9 @@ export async function getFreshOAuth2Credentials(
}
}

function makeUrl(template: string, config: Record<string, any>, encodeAllParams = true): URL {
function makeUrl(template: string, config: Record<string, any>, skipEncodeKeys: string[] = []): URL {
const cleanTemplate = template.replace(/connectionConfig\./g, '');
const encodedParams = encodeAllParams ? encodeParameters(config) : config;
const encodedParams = skipEncodeKeys.includes('base_url') ? config : encodeParameters(config);
const interpolatedUrl = interpolateString(cleanTemplate, encodedParams);
return new URL(interpolatedUrl);
}
5 changes: 4 additions & 1 deletion packages/shared/providers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2591,7 +2591,8 @@ github-app-oauth:
alias: github
auth_mode: CUSTOM
authorization_url: ${connectionConfig.appPublicLink}/installations/new
authorization_url_encode: false
authorization_url_skip_encode:
- base_url
token_url:
OAUTH2: https://github.com/login/oauth/access_token
APP: https://api.github.com/app/installations/${connectionConfig.installation_id}/access_tokens
Expand Down Expand Up @@ -4469,6 +4470,8 @@ pennylane:
grant_type: authorization_code
refresh_params:
grant_type: refresh_token
authorization_url_skip_encode:
- scopes
docs: https://docs.nango.dev/integrations/all/pennylane

peopledatalabs:
Expand Down
4 changes: 2 additions & 2 deletions packages/types/lib/providers/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ export interface BaseProvider {
};
};
authorization_url?: string;
authorization_url_encode?: boolean;
authorization_url_skip_encode?: string[];
access_token_url?: string;
authorization_params?: Record<string, string>;
scope_separator?: string;
default_scopes?: string[];
token_url?: string | TokenUrlObject;
token_url_encode?: boolean;
token_url_skip_encode?: string[];
token_params?: Record<string, string>;
authorization_url_replacements?: Record<string, string>;
redirect_uri_metadata?: string[];
Expand Down
14 changes: 10 additions & 4 deletions scripts/validation/providers/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,11 @@
"authorization_url": {
"type": "string"
},
"authorization_url_encode": {
"type": "boolean"
"authorization_url_skip_encode": {
"type": "array",
"items": {
"type": "string"
}
},
"authorization_url_replacements": {
"type": "object",
Expand Down Expand Up @@ -397,8 +400,11 @@
}
]
},
"token_url_encode": {
"type": "boolean"
"token_url_skip_encode": {
"type": "array",
"items": {
"type": "string"
}
},
"webhook_user_defined_secret": {
"type": "boolean"
Expand Down

0 comments on commit 57bac9a

Please sign in to comment.