Expose identity token for oauth2 #3211

sayoojs · 2021-03-19T11:14:45Z

Exposed identity token in the UI for all grant types in Oauth2.

Shows both identity token and refresh token in the Ctrl+Space autocomplete list for ease of use.

Made sure to handle populating identity token on clicking the Refresh Token button.

vercel · 2021-03-19T11:14:49Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/teaminsomnia/insomnia-storybook/6RWqWc3Kfcrud3HMf2XmKo97aX3E
✅ Preview: Canceled

[Deployment for ff90aef canceled]

… type

develohpanda

Generally looks good but a couple of changes to make here 😄

I am also not set up to be able to QA this, if you could outline some of your QA steps to show this works and fails correctly that would be very much appreciated.

packages/insomnia-app/app/ui/components/editors/auth/o-auth-2-auth.js

plugins/insomnia-plugin-request/index.js

vercel · 2021-03-22T06:00:38Z

@sayoojs is attempting to deploy a commit to the Insomnia Team on Vercel.

A member of the Team first needs to authorize it.

sayoojs · 2021-03-22T06:20:50Z

@develohpanda The QA I've done so far is

With 'Grant Type' as Authorization Code, tried querying an endpoint protected by AWS Cognito user pools which uses the identityToken as Bearer. Successfully retrieved a 200.
In cases where there is no identityToken returned by the auth provider, the input field will show n/a, and 'No token' message is shown on auto complete(Ctrl+Space).(Emulated the null return by manually removing the return for this attribute from the get function)
- In case of either an empty string value or a null. Checked if the 'No token' message showed up on the autocomplete tag's edit screen.
Have NOT tested any of the other grant types due to not having any existing environments which uses those authentication grant methods at hand. This is a concern.

I've also made the requested changes and pushed the commit, but since I forked, Vercel, is asking for authentication.

develohpanda

Thanks for the quick turnaround and your notes! 🙌🏽

I am happy with the code and usability though I'm not sure about the expectation for token expiry (should it be the same as the access token?). We may also need to explore how to QA this, maybe allowing users from the parent issue to QA a beta build.

@DMarby may have some thoughts on QA as well.

plugins/insomnia-plugin-request/index.js

sayoojs · 2021-03-26T11:00:12Z

Hi @develohpanda

I've pushed another commit which addresses handling the Identity Token expiry separately.
What I've done is decode the identityToken JWT to fetch the exp and render the expiry from that value.
So on the off chance the expiry for the identity token is different from that of the access token, it should render differently.

Also, any updates on including this in a beta build?

Thanks in advance! 😃

develohpanda · 2021-03-26T11:21:47Z

Awesome! I will review again, and I'll raise it with the team next week to make sure we get some more reviews on this PR and figure out a way forward. 🤗

develohpanda

Nice 🎉

develohpanda · 2021-03-30T09:52:20Z

packages/insomnia-app/app/ui/components/editors/auth/o-auth-2-auth.js

+      make it exactly 13 digits. As the 'TimeFromNow' component only seems to 
+      render 13 digit epochs correctly.
+    */
+    const expDigitCount = exp.toString().length;


Looks like it's using moment to create the timestamp.

What does the expiry extracted from the token look like/represent? Feels like the padding or removing of digits could cause some strange behavior 🤔

The extracted expiry for identityToken was a 10 digit epoch representation in my case (Eg: 1617202697 Precision up to seconds).
For accessToken's expires_in key its 13 digit epoch (Eg: 1617202697111 Precision up to milliseconds).

I see! Reading some docs about the JWT exp property it seems this is usually in seconds. Is this always the case or situational? If always, I'd be more comfortable with a solution what converts seconds to milliseconds by multiplying by 1000, rather than removing or padding digits 🤔 since we're working with numbers the multiplication seems easier to follow.

Oh no, by padding (as shown in snippet below) that's what I'm doing.

I'm essentially dividing or multiplying by 10^n, where n is calculated as the difference between the number 13 and the actual digit count of epoch generated from the JWT exp property.

exp = exp * 10 ** (13 - expDigitCount);

So in cases of epoch seconds, that code multiplies by 10^3.
In case of epoch microseconds, that code multiplies by 10^-3. Which essentially remove the digits that representing the microseconds.

Maybe I should force cast the result to big int to prevent floating points on x10^-n

Ahhhh okay, thanks for explaining that!!

Maybe I should force cast the result to big int to prevent floating points on x10^-n

That sounds reasonable.

I think it would be good to extract these few lines into a standalone utility method somewhere and writing a few tests as a way to document input/output of this logic. 🙌🏽

Which lines specifically? The padding/trimming part?

Yep! The padding and trimming

Is there a file with preexisting util functions? Where I can export this to? Or do I just create a static method within this class itself?

This might be a good place! Looks like there a similar helper here too, for snapping a number to a minimum or maximum.

insomnia/packages/insomnia-app/app/common/misc.js

Line 444 in 45c210e

export function snapNumberToLimits(value: number, min?: number, max?: number): number {

The associated tests would go in here: https://github.com/Kong/insomnia/blob/develop/packages/insomnia-app/app/common/__tests__/misc.test.js

Moved this functionality to misc.js and added tests. (Do review the tests, this is the first time I've written JS tests)

develohpanda

There is a bug here such that window.atob() fails fatally if the identity token is not encoded correctly (if you just type anything into the input field). The field has a requirement to be encoded in a particular way from what I can see, which is fine, but there shouldn't be a UI failure if it's not encoded as expected. 🤔

Another bug in comments below. Once these are fixed up I can trigger an alpha build, or you can bundle it locally for your OS by running npm run app-package:smoke and fetching artefacts from the packages/insomnia-app/dist folder.

develohpanda · 2021-03-30T23:17:46Z

packages/insomnia-app/app/ui/components/editors/auth/o-auth-2-auth.js

+  static renderExpiryFromJWT(token: string): React.Element<*> | string | null {
+    if (!token) {
+      return null;
+    }
+
+    const base64Url = token.split('.')[1];


I just pulled this down locally to test and the Oauth2 tab fails to render for a brand new request as a result of this implementation. The following suggestion matches how renderExpireAt is written

Suggested change

static renderExpiryFromJWT(token: string): React.Element<*> | string | null {

if (!token) {

return null;

}

const base64Url = token.split('.')[1];

static renderExpiryFromJWT(token: ?OAuth2Token): React.Element<*> | string | null {

if (!token || !token.identitiyToken) {

return null;

}

const base64Url = token.identitiyToken.split('.')[1];

I'll make this change.
I was trying to keep the function generic, to be able to be used with any JWT 😅
I guess since accessToken already has an expiry function, i'll make this one dedicated to identityToken.

Fair enough 😅 it fails because likely somewhere in the render loop this logic is reached while token is undefined, and the previous code would fail with an error like identityToken cannot be found on undefined

develohpanda · 2021-03-30T23:18:31Z

packages/insomnia-app/app/ui/components/editors/auth/o-auth-2-auth.js

@@ -541,7 +580,8 @@ class OAuth2Auth extends React.PureComponent<Props, State> {
    const { request, oAuth2Token: tok } = this.props;
    const { loading, error, showAdvanced } = this.state;

-    const expireLabel = OAuth2Auth.renderExpireAt(tok);
+    const accessExpireLabel = OAuth2Auth.renderExpireAt(tok);
+    const identityExpireLabel = OAuth2Auth.renderExpiryFromJWT(tok.identityToken);


Suggested change

const identityExpireLabel = OAuth2Auth.renderExpiryFromJWT(tok.identityToken);

const identityExpireLabel = OAuth2Auth.renderExpiryFromJWT(tok);

This fix is implemented.

sayoojs · 2021-04-05T11:30:25Z

@develohpanda I've implemented the requested changes.

Any idea why the Ubuntu tests might be failing? I couldn't infer anything from the logs. Could you try re-triggering it?

develohpanda

Thank you so much 🙏🏽 Sometimes Spectron (the E2E test framework we use for Electron) fails to connect to the chrome driver and gives cryptic errors like that. I have re-triggered, no action needed from your side 🤗

kateefimova-eb · 2022-03-14T17:14:14Z

What an amazing feature! Thank you so much 👏

Kong#3211 Allowed for using an identity token for auth rather than auth token. When using that feature, the GraphQL schema could not be fetched due to `throw new Error('No OAuth 2.0 identity tokens found for request');`. This commit moves the normalization of the request ID further up the chain so it can be used where this error would otherwise be thrown.

sayoojs added 4 commits March 19, 2021 16:00

Exposed identity token in the UI for oauth2.0.

610a467

Shows identity token on ctrl+space auto complete.

afb07a9

Renamed consts

c95322a

Show refreshToken as a ctrl+shift autocomplete option.

2139db5

vercel bot temporarily deployed to Preview March 19, 2021 11:14 Inactive

sayoojs added 2 commits March 19, 2021 17:14

Updated tests

ccc6970

Removed the line which deletes the id_token in case of implicit grant…

ff90aef

… type

vercel bot temporarily deployed to Preview March 19, 2021 11:46 Inactive

develohpanda self-requested a review March 19, 2021 20:11

develohpanda suggested changes Mar 22, 2021

View reviewed changes

packages/insomnia-app/app/ui/components/editors/auth/o-auth-2-auth.js Outdated Show resolved Hide resolved

plugins/insomnia-plugin-request/index.js Outdated Show resolved Hide resolved

Made changes as per review

b628975

Merge branch 'develop' into expose-identity-token-for-oauth2

e69f280

vercel bot temporarily deployed to Preview March 22, 2021 06:24 Inactive

develohpanda approved these changes Mar 22, 2021

View reviewed changes

plugins/insomnia-plugin-request/index.js Show resolved Hide resolved

develohpanda requested a review from DMarby March 22, 2021 06:52

Merge branch 'develop' into expose-identity-token-for-oauth2

e82f6fa

vercel bot temporarily deployed to Preview March 23, 2021 13:03 Inactive

Merge branch 'develop' into expose-identity-token-for-oauth2

b5fa479

vercel bot temporarily deployed to Preview March 24, 2021 12:23 Inactive

Calculate identity token expiry seperately.

b7d9b4f

vercel bot temporarily deployed to Preview March 26, 2021 10:52 Inactive

Merge branch 'develop' into expose-identity-token-for-oauth2

efe7063

vercel bot temporarily deployed to Preview March 26, 2021 10:53 Inactive

sayoojs requested a review from develohpanda March 26, 2021 11:04

Removed a console log

041345b

vercel bot temporarily deployed to Preview March 26, 2021 11:08 Inactive

develohpanda approved these changes Mar 30, 2021

View reviewed changes

develohpanda requested a review from dimitropoulos March 30, 2021 21:12

Merge branch 'develop' into expose-identity-token-for-oauth2

f3e181a

vercel bot temporarily deployed to Preview March 30, 2021 21:59 Inactive

develohpanda suggested changes Mar 30, 2021

View reviewed changes

Made changes to fix window.atob() failure case

0ebfc55

vercel bot temporarily deployed to Preview April 1, 2021 09:07 Inactive

Merge branch 'develop' into expose-identity-token-for-oauth2

7f5b692

vercel bot temporarily deployed to Preview April 1, 2021 09:13 Inactive

Moved epoch conversion to misc.js and added tests.

5361c0f

vercel bot temporarily deployed to Preview April 5, 2021 11:06 Inactive

sayoojs requested a review from develohpanda April 5, 2021 11:26

develohpanda approved these changes Apr 5, 2021

View reviewed changes

Merge branch 'develop' into expose-identity-token-for-oauth2

5c319f9

vercel bot temporarily deployed to Preview April 5, 2021 16:28 Inactive

dimitropoulos merged commit 351afde into Kong:develop Apr 5, 2021

danelowe mentioned this pull request Jun 13, 2022

Allow using ID Token to retrieve schema #4857

Closed

doronguttman mentioned this pull request Jan 27, 2023

[Feature Request] OAuth 2 missing Identity token. #1423

Closed

Expose identity token for oauth2 #3211

Expose identity token for oauth2 #3211

sayoojs commented Mar 19, 2021

vercel bot commented Mar 19, 2021 •

edited

develohpanda left a comment •

edited

vercel bot commented Mar 22, 2021

sayoojs commented Mar 22, 2021 •

edited

develohpanda left a comment

sayoojs commented Mar 26, 2021

develohpanda commented Mar 26, 2021

develohpanda left a comment

develohpanda Mar 30, 2021

sayoojs Mar 31, 2021

develohpanda Apr 1, 2021

sayoojs Apr 1, 2021

develohpanda Apr 1, 2021

sayoojs Apr 1, 2021

develohpanda Apr 1, 2021

sayoojs Apr 1, 2021

develohpanda Apr 1, 2021 •

edited

sayoojs Apr 5, 2021 •

edited

develohpanda left a comment •

edited

develohpanda Mar 30, 2021

sayoojs Mar 31, 2021

develohpanda Apr 1, 2021

develohpanda Mar 30, 2021

sayoojs Apr 5, 2021

sayoojs commented Apr 5, 2021

develohpanda left a comment

kateefimova-eb commented Mar 14, 2022

	const identityExpireLabel = OAuth2Auth.renderExpiryFromJWT(tok.identityToken);
	const identityExpireLabel = OAuth2Auth.renderExpiryFromJWT(tok);

Expose identity token for oauth2 #3211

Expose identity token for oauth2 #3211

Conversation

sayoojs commented Mar 19, 2021

vercel bot commented Mar 19, 2021 • edited

develohpanda left a comment • edited

Choose a reason for hiding this comment

vercel bot commented Mar 22, 2021

sayoojs commented Mar 22, 2021 • edited

develohpanda left a comment

Choose a reason for hiding this comment

sayoojs commented Mar 26, 2021

develohpanda commented Mar 26, 2021

develohpanda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

develohpanda Apr 1, 2021 • edited

Choose a reason for hiding this comment

sayoojs Apr 5, 2021 • edited

Choose a reason for hiding this comment

develohpanda left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sayoojs commented Apr 5, 2021

develohpanda left a comment

Choose a reason for hiding this comment

kateefimova-eb commented Mar 14, 2022

vercel bot commented Mar 19, 2021 •

edited

develohpanda left a comment •

edited

sayoojs commented Mar 22, 2021 •

edited

develohpanda Apr 1, 2021 •

edited

sayoojs Apr 5, 2021 •

edited

develohpanda left a comment •

edited