OAuth2 Revamp #3867

helloanoop · 2025-01-23T11:16:34Z

OAuth2 Revamp

…ad of api endpoint (#1999) Setting oauth2 authorization no longer equals overwriting user-specified data in a request. The pre-requests made to obtain oauth2 access_token are now separated from actual API request.

Results of oauth2 authorization flow (i.e. access_token but also refresh_token, id_token, scope or any other information returned from token request) are stored in a collection specific cache. It is persisted in the file system, and will be automatically reused when executing requests until the cache is purged (using Clear Cache button available in all related views).

…o be usable by scripts The new variable 'credentials' is now available in 'req' object. It is added automatically during request preparation if oauth2 method is used and is value is either evaluated or retrieved from collection oauth2 cache.

…vel Get Access Token action The actual the authorization request is now part of request preparation, and its response is returned for post-request script processing.

According to RFC6749 Section 7.1, The client MUST NOT use an access token if it does not understand the token type. At this point bruno only understands 'bearer' token_type.

…utput pane fix: typo - rename OAuth2PasswordCredentials component fix: typo - Use the same name for AuthMode - OAuth 2.0 in collection and request level

pietrygamat · 2025-01-23T23:52:37Z

The UI here looks great and token decoder is way ahead of what the competition offers, so make sure to post a promotional video after releasing this!
🥇

pietrygamat · 2025-01-23T21:32:22Z

packages/bruno-app/src/components/RequestPane/Auth/OAuth2/AuthorizationCode/StyledWrapper.js

@@ -11,6 +11,47 @@ const Wrapper = styled.div`
    border: solid 1px ${(props) => props.theme.input.border};
    background-color: ${(props) => props.theme.input.bg};
  }
+
+  .token-placement-selector {


How about extracting token placement with corresponding HeaderPrefix/QueryParam field into a separate component? Between CSS, change handlers, dropdownRefs and currently hardcoded translatable strings, it's a lot of duplication.

pietrygamat · 2025-01-23T21:36:39Z

packages/bruno-app/src/components/RequestPane/Auth/OAuth2/GrantTypeSelector/index.js

+            tokenPrefix: 'Bearer',
+            tokenQueryParamKey: 'access_token',


Both of these default values are saved in bru file, even if the tokenPlacement option only needs one. It would be better if unused values are not automatically pushed into request configurations.

pietrygamat · 2025-01-23T21:40:19Z

packages/bruno-app/src/components/RequestPane/Auth/OAuth2/Oauth2TokenViewer/index.js

+const ExpiryTimer = ({ initialExpiresIn }) => {
+  const [timeLeft, setTimeLeft] = useState(initialExpiresIn);


This timer resets when changing tabs.

pietrygamat · 2025-01-23T21:45:31Z

packages/bruno-app/src/components/RequestPane/Auth/OAuth2/AuthorizationCode/index.js

+  const handleFetchOauth2Credentials = async () => {
+    let requestCopy = cloneDeep(request);
+    requestCopy.oauth2 = requestCopy?.auth.oauth2;
+    requestCopy.headers = {};
+    toggleFetchingToken(true);
+    try {
+      await dispatch(fetchOauth2Credentials({ request: requestCopy, collection }));
+      toggleFetchingToken(false);
+    }
+    catch(error) {
+      console.error('could not fetch the token!');
+      console.error(error);
+      toggleFetchingToken(false);
+    }
+  }


Indicate a token fetch success with a toast?

pietrygamat · 2025-01-23T21:48:51Z

packages/bruno-app/src/components/RequestPane/Auth/OAuth2/AuthorizationCode/index.js

      {inputsConfig.map((input) => {
        const { key, label, isSecret } = input;
        return (


Please, make this section collapsible. The Oauth2TokenViewer is so pretty to look at, the users may want to have in view all the time, but they may sometimes have to scroll down to the Get Token button.

pietrygamat · 2025-01-23T21:58:18Z

packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js

+    ipcRenderer.invoke('clear-oauth2-cache', collectionUid, url, credentialsId).then(resolve => {
+      dispatch(collectionClearOauth2CredentialsByUrl({ collectionUid, url, credentialsId }));
+      resolve();


I'm getting a toast: Resolve is not a function.

pietrygamat · 2025-01-23T22:06:45Z

packages/bruno-app/src/components/RequestPane/Auth/OAuth2/AuthorizationCode/index.js

+          credentialsId,
+          tokenPlacement,
+          tokenPrefix,
+          tokenQueryParamKey,
+          reuseToken,


There's a lot of places where the new fields need keeping track of. It seems it can be replaced with simply:

dispatch(updateAuth({ mode: 'oauth2', collectionUid: collection.uid, itemUid: item.uid, content: { ...oAuth, [key]: value } }))

pietrygamat · 2025-01-23T22:17:14Z

packages/bruno-app/src/components/RequestPane/Auth/OAuth2/AuthorizationCode/index.js

+              className="dropdown-item"
+              onClick={() => {
+                dropdownTippyRef.current.hide();
+                handleChange('tokenPlacement', 'header');


Can these enum-like values: header, url be externalized somehow? They are used in views, in bru files, schema definitions, soon - as keys in translation files. Maybe this refactor is a good opportunity to take care of that as well?

pietrygamat · 2025-01-23T22:59:05Z

packages/bruno-electron/src/utils/oauth2.js

+  requestCopy.url = url;
+
+  const axiosInstance = makeAxiosInstance();
+  const response = await axiosInstance(requestCopy);


This request fails due to certificate validation issues (expired certificate), despite having disabled SSL/TLS Certificate verification. Note, the electron browser authentication does work (login prompt is shown). The problem is also that it fails silently - no toast, no timeline information about the cause.

pietrygamat · 2025-01-23T23:43:56Z

packages/bruno-electron/src/utils/oauth2.js

+  requestCopy.url = url;
+
+  const axiosInstance = makeAxiosInstance();
+  const response = await axiosInstance(requestCopy);


This request is sent with Header:

Authorization: 'Basic Og=='

There are several places in the code like this, where any auth is confused with Basic auth resulting in storing base64 representation of ":" in Authorization header.

// todo: we have things happening in two places w.r.t basic auth // need to refactor this in the future // the request.auth (basic auth) object gets set inside the prepare-request.js file if (request.auth) { const username = _interpolate(request.auth.username) || ''; const password = _interpolate(request.auth.password) || ''; // use auth header based approach and delete the request.auth object request.headers['Authorization'] = `Basic ${Buffer.from(`${username}:${password}`).toString('base64')}`; delete request.auth; }

Before implementing #2106 this is critical, because many servers will look at Basic Auth rather than post body for client credentials, which are part of the OAuth2 flow. I think this refactor is also the time to resolve these TODOs.

~ basic auth credentials should be assigned to `request.basicAuth` instead `request.auth` object ~ added credentials_placement option, fixed headers issue client credentials flow ~ cache input field values when grant type select box value changes ~ updated logic for - cache input field values when grant type select box value changes ~ updated token expiry timer component logic

~ changed tokenPrefix to tokenHeaderPrefix ~ updated the logic for token timer component

lohit-bruno · 2025-01-24T14:27:07Z

thanks for the review @pietrygamat !

pietrygamat · 2025-01-25T22:21:29Z

packages/bruno-electron/src/utils/oauth2.js

+      client_id: clientId,
+      client_secret: clientSecret


Only

if (credentialsPlacement === "body"

pietrygamat · 2025-01-25T22:43:41Z

packages/bruno-app/src/components/RequestPane/Auth/OAuth2/PasswordCredentials/index.js

+            <div
+              className="dropdown-item"
+              onClick={() => {
+                dropdownTippyRef.current.hide();
+                handleChange('credentialsPlacement', 'basic_auth_header');
+              }}
+            >
+              Basic Auth Header
+            </div>


Sending client creds in Authorization header is the preferred choice (servers MUST support it), and including them in request body is optional alternative (servers MAY support it). So it would make sense for Bruno to default to basic_auth_header and have it as a first option.

pietrygamat and others added 8 commits September 23, 2024 20:50

feat: OAuth2 - Do not make axios request when executing collection le…

d982e35

…vel Get Access Token action The actual the authorization request is now part of request preparation, and its response is returned for post-request script processing.

feat: OAuth2 - automatically handle Bearer token type only

2064cc8

According to RFC6749 Section 7.1, The client MUST NOT use an access token if it does not understand the token type. At this point bruno only understands 'bearer' token_type.

feat: OAuth2 - UI for OAuth2 Credentials independent of the Request O…

dd9cb21

…utput pane fix: typo - rename OAuth2PasswordCredentials component fix: typo - Use the same name for AuthMode - OAuth 2.0 in collection and request level

feat: OAuth2 - Supported at the collection level (#1704)

3bd8f09

Merge branch 'pr-2077' into feat/oauth2-improvements

8f75414

pull-request-size bot added the size/L label Jan 23, 2025

helloanoop changed the title ~~Feat/oauth2 improvements~~ OAuth2 Revamp Jan 23, 2025

pietrygamat mentioned this pull request Jan 23, 2025

fix (#1999) request url overwritten #2061

Closed

5 tasks

oauth2 ui/ux improvements (#3868)

f38c7ae

pull-request-size bot added size/XXL and removed size/L labels Jan 23, 2025

pietrygamat suggested changes Jan 23, 2025

View reviewed changes

lohxt1 added 2 commits January 24, 2025 18:44

oauth2 updates (#3876)

c5eeb19

~ changed tokenPrefix to tokenHeaderPrefix ~ updated the logic for token timer component

pietrygamat suggested changes Jan 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OAuth2 Revamp #3867

OAuth2 Revamp #3867

helloanoop commented Jan 23, 2025

pietrygamat commented Jan 23, 2025

pietrygamat Jan 23, 2025

pietrygamat Jan 23, 2025 •

edited

Loading

pietrygamat Jan 23, 2025

pietrygamat Jan 23, 2025

pietrygamat Jan 23, 2025

pietrygamat Jan 23, 2025

pietrygamat Jan 23, 2025

pietrygamat Jan 23, 2025

pietrygamat Jan 23, 2025 •

edited

Loading

pietrygamat Jan 23, 2025

lohit-bruno commented Jan 24, 2025

pietrygamat Jan 25, 2025

pietrygamat Jan 25, 2025

		const ExpiryTimer = ({ initialExpiresIn }) => {
		const [timeLeft, setTimeLeft] = useState(initialExpiresIn);

OAuth2 Revamp #3867

Are you sure you want to change the base?

OAuth2 Revamp #3867

Conversation

helloanoop commented Jan 23, 2025

pietrygamat commented Jan 23, 2025

Choose a reason for hiding this comment

pietrygamat Jan 23, 2025 • edited Loading

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

pietrygamat Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lohit-bruno commented Jan 24, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pietrygamat Jan 23, 2025 •

edited

Loading

pietrygamat Jan 23, 2025 •

edited

Loading