-
Notifications
You must be signed in to change notification settings - Fork 18
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: OAuth 2.0 functionality #290
base: master
Are you sure you want to change the base?
Changes from all commits
31787ae
8a3e48e
a334a20
438cd91
3889280
612b7a1
47ee0b8
9c4066b
a747827
98a23b0
b3fe128
a98f43a
efcfe27
68ba371
0c43ea9
9f02dc9
c45bc8e
1f97cc9
369209c
56ad9b6
7cdd1d4
274e7b7
6900756
3b5069e
7071e1c
de85e68
81c2c03
341b32c
f6b8697
0d56460
49fe426
abbc152
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,9 +37,9 @@ export default class Endpoint { | |
client: Client; | ||
options: CommandOptions; | ||
|
||
constructor(client: Client, options: CommandOptions) { | ||
constructor(client: Client, options?: CommandOptions) { | ||
this.client = client; | ||
this.options = options; | ||
this.options = options ? options : this.client.options; | ||
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. Not really sure if that's the best idea, but test are feeling fine. |
||
} | ||
|
||
configureRequest<T>(requestName: string, config: RequestConfig<T>): T { | ||
|
@@ -109,7 +109,15 @@ export default class Endpoint { | |
const { customHostname, raw, onProgress } = apiOptions; | ||
const hostname = customHostname || (await this.options.apiUrl); | ||
|
||
fetchOptions.body = fetchOptions.body && JSON.stringify(fetchOptions.body); | ||
if ( | ||
!fetchOptions.headers || | ||
fetchOptions.headers["Content-Type"] !== | ||
"application/x-www-form-urlencoded" | ||
) { | ||
fetchOptions.body = | ||
fetchOptions.body && JSON.stringify(fetchOptions.body); | ||
} | ||
|
||
fetchOptions.headers = await this._getFetchHeaders(fetchOptions.headers); | ||
const args = [`${hostname}/${url}`, fetchOptions]; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
// @flow | ||
import { BaseError } from "../errors"; | ||
import type { | ||
OAuthAuthorizeInput, | ||
OAuthTokenInput, | ||
TokenResponseData | ||
} from "../types"; | ||
import Endpoint from "../endpoints/Endpoint"; | ||
|
||
export default class OAuth extends Endpoint { | ||
name = "oauth"; | ||
|
||
getToken(input: OAuthTokenInput) { | ||
const clientId = input.clientId || this.options.clientId; | ||
const clientSecret = input.clientSecret || this.options.clientSecret; | ||
const redirectUri = input.redirectUri || this.options.redirectUri; | ||
const { authorizationCode } = input; | ||
|
||
const body = new URLSearchParams(); | ||
|
||
if (!clientId || !clientSecret || !redirectUri || !authorizationCode) { | ||
throw new Error("OAuthTokenInput required"); | ||
} | ||
|
||
body.append("client_id", clientId); | ||
body.append("client_secret", clientSecret); | ||
body.append("redirect_uri", redirectUri); | ||
|
||
body.append("code", authorizationCode); | ||
body.append("grant_type", "authorization_code"); | ||
|
||
return this.configureRequest<Promise<TokenResponseData>>("getToken", { | ||
api: async () => { | ||
const response = await this.apiRequest( | ||
`auth/tokens`, | ||
{ | ||
method: "POST", | ||
headers: { | ||
"Content-Type": "application/x-www-form-urlencoded" | ||
}, | ||
body | ||
}, | ||
{ | ||
customHostname: "https://auth.goabstract.com" | ||
} | ||
); | ||
|
||
return response.access_token; | ||
} | ||
}); | ||
} | ||
|
||
generateAuthorizeUrl(input: OAuthAuthorizeInput): string { | ||
const clientId = input.clientId || this.options.clientId; | ||
const state = input.state; | ||
const redirectUri = input.redirectUri || this.options.redirectUri; | ||
const scope = input.scope || "all"; | ||
|
||
if (!clientId || !redirectUri) { | ||
throw new BaseError( | ||
"Client credentials are missing. Please double check clientId, redirectUri and state" | ||
); | ||
} | ||
|
||
return `https://app.abstract.com/signin/auth/authorize?client_id=${clientId}&redirect_uri=${encodeURIComponent( | ||
redirectUri | ||
)}&response_type=code&scope=${scope}&state=${state}`; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -159,9 +159,12 @@ export type CommandOptions = { | |
accessToken?: AccessTokenOption, | ||
analyticsCallback: AnalyticsCallback, | ||
apiUrl: string | Promise<string>, | ||
clientId?: string, | ||
clientSecret?: string, | ||
objectUrl: string | Promise<string>, | ||
previewUrl: string | Promise<string>, | ||
shareId?: () => Promise<string | ShareDescriptor | ShareUrlDescriptor | void>, | ||
redirectUri?: string, | ||
transportMode: ("api" | "cli")[], | ||
webUrl: string | Promise<string> | ||
}; | ||
|
@@ -1672,3 +1675,25 @@ export type ReviewRequest = { | |
status: ReviewStatus, | ||
statusChangedAt: string | ||
}; | ||
|
||
export type OAuthAuthorizeInput = { | ||
clientId: string, | ||
redirectUri: string, | ||
state: string | ||
}; | ||
|
||
export type OAuthTokenInput = { | ||
redirectUri: string, | ||
clientSecret: string, | ||
clientId: string, | ||
authorizationCode: string | ||
}; | ||
|
||
export type TokenResponseData = { | ||
access_token: string, | ||
client_id: string, | ||
created_at: string, | ||
id: string, | ||
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. What is this 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. According to back end, it's not used anywhere right now and not much needed for client. Looks like an id in database. |
||
scope: string, | ||
user_id: string | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
import { mockAuth, API_CLIENT } from "../../src/util/testing"; | ||
|
||
describe("oauth", () => { | ||
describe("getToken", () => { | ||
const [clientId, clientSecret, redirectUri, authorizationCode] = [ | ||
"client_id", | ||
"client_secret", | ||
"redirect_uri", | ||
"authorization_code" | ||
]; | ||
test("api - with data", async () => { | ||
mockAuth( | ||
"/auth/tokens", | ||
{ | ||
access_token: "access_token", | ||
client_id: "client_id", | ||
created_at: "created_at", | ||
id: "id", | ||
scope: "scope", | ||
user_id: "user_id" | ||
}, | ||
200, | ||
"post" | ||
); | ||
|
||
const response = await API_CLIENT.oauth.getToken({ | ||
clientId: "client_id", | ||
clientSecret: "client_secret", | ||
redirectUri: "redirect_uri", | ||
authorizationCode: "authorization_code" | ||
}); | ||
|
||
expect(response).toEqual("access_token"); | ||
}); | ||
|
||
test("api - without clientId", async () => { | ||
expect(() => | ||
API_CLIENT.oauth.getToken({ | ||
clientSecret, | ||
redirectUri, | ||
authorizationCode | ||
}) | ||
).toThrowError(); | ||
}); | ||
|
||
test("api - without clientSecret", async () => { | ||
expect(() => | ||
API_CLIENT.oauth.getToken({ | ||
clientId, | ||
redirectUri, | ||
authorizationCode | ||
}) | ||
).toThrowError(); | ||
}); | ||
|
||
test("api - without redirectUri", async () => { | ||
expect(() => | ||
API_CLIENT.oauth.getToken({ | ||
clientId, | ||
clientSecret, | ||
authorizationCode | ||
}) | ||
).toThrowError(); | ||
}); | ||
|
||
test("api - without authorizationCode", async () => { | ||
expect(() => | ||
API_CLIENT.oauth.getToken({ | ||
clientId, | ||
clientSecret, | ||
redirectUri | ||
}) | ||
).toThrowError(); | ||
}); | ||
}); | ||
|
||
describe("generateAuthUrl", () => { | ||
test("options are passed", () => { | ||
const [clientId, redirectUri, state] = [ | ||
"clientId", | ||
"redirectUri", | ||
"state" | ||
]; | ||
|
||
const url = API_CLIENT.oauth.generateAuthorizeUrl({ | ||
clientId, | ||
redirectUri, | ||
state | ||
}); | ||
|
||
expect(url).toEqual( | ||
`https://app.abstract.com/signin/auth/authorize?client_id=${clientId}&redirect_uri=${redirectUri}&response_type=code&scope=all&state=${state}` | ||
); | ||
}); | ||
|
||
test("options are not passed", () => { | ||
expect(() => API_CLIENT.oauth.generateAuthorizeUrl({})).toThrowError(); | ||
}); | ||
}); | ||
}); |
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.
I think we should go ahead and just change
Endpoint
to only accept one argument,this
– we can readthis.options
in the constructor and avoid passing it in essentially twice here.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.
Done! I left Endpoint options param as optional, because tests were behaving weird.