Add Support for Postman Authorization via API Key #52

sl4wa · 2024-07-13T09:25:36Z

Summary

This PR adds support for Postman API key authorization via a new console parameter --AuthHeader="header: value" or -A="header: value".

Changes

cli.ts: Added parsing for the new --AuthHeader (-A) parameter.
index.ts: Modified createPostmanCollection to accept API key header and value, handling the global authorization header separately and filtering it out from individual request headers.
format.ts: Updated queryCollectionToPostmanCollection and queryToItem to include the API key authorization block in the Postman collection and ensure the API key header is not added to individual items.
lib.ts: Refactored parseHeaders to handle the --AuthHeader (-A) parameter correctly, returning an array of [string, string] tuples.

Testing

Run the CLI with the new --AuthHeader (-A) parameter.
Verify that the generated Postman collection includes the API key authorization block at the root level and excludes the API key header from individual items.

Additional Notes

Please review the changes and let me know if any adjustments are needed.

nohehf · 2024-07-22T11:51:54Z

Hey there ! Thanks for the PR :)
Could you give more context / a specific use-case for why/when this extra flag is needed ? I feel like it is a bit mixed up with the "standard" headers (-H flag).
Thanks

sl4wa · 2024-07-22T13:42:44Z

Here is the use case.

Instead of having header being added to every HTTP request in the collection, Postman supports global level authorization.

For example by API Key.

This key will be added to the requests within the collection.

nohehf

Okay, that's what I thought thanks for the addition and suggestion.
After looking at the docs: https://learning.postman.com/docs/sending-requests/authorization/authorization-types/, it seems it could support most of the use cases so I think it's a nice addition.
I added some comments and suggestions, to harmonize it all. Graphman is quite an old codebase now, but I think it would be nice this way.
Sorry for the delay of my review, I had a lot of work lately :))
Also can you please add the flag usage to README.md?
Thank you!

nohehf · 2024-07-26T07:42:44Z

src/cli.ts

+ combinedHeaders.push(args.Hapikey);
+}
+
+const headers = parseHeaders(combinedHeaders);


I don't think you want to mix the global header and regular ones, as I can see you later filter it out. Please juste parse and pass those separately

adding the combined header because they should be used in api call, right?

nohehf · 2024-07-26T07:43:47Z

src/format.ts

 }

 function queryToItem(
 query: FormattedQuery,
 url: string,
 headers?: Record<string, string>,
+ apiKeyHeader?: string,


If you separate them as suggested, you can just remove it from this signature

nohehf · 2024-07-26T07:44:01Z

src/format.ts

- value,
- })),
+ header: Object.entries(headers ?? {})
+ .filter(([key]) => key !== apiKeyHeader)


Idem you won't need to filter it out

nohehf · 2024-07-26T07:45:45Z

src/format.ts

@@ -83,17 +94,24 @@ export function queryCollectionToPostmanCollection(
 queryCollection: QueryCollection,
 url: string,
 headers?: Record<string, string>,
-) {
+ apiKeyHeader?: string,


Pass the global header as a tuple[string,string]

nohehf · 2024-07-26T07:46:20Z

src/format.ts

+ value: string;
+ type: string;
+ }[];
+ };
 }

 function queryToItem(
 query: FormattedQuery,
 url: string,
 headers?: Record<string, string>,


We should change this type to Array<[string,string]>, and make it mandatory (it will just be empty if no headers).

nohehf · 2024-07-26T07:46:46Z

src/index.ts

@@ -8,14 +8,24 @@ export { outrospect };
 export async function createPostmanCollection(
 url: string,
 headers?: Record<string, string>,
+ apiKeyHeader?: string,


idem tuple here

nohehf · 2024-07-26T07:47:05Z

src/lib.ts

- `\n\nError parsing: \n ${h}. \n Please verify your headers.\n`,
- );
- }
+export function parseHeaders(headers?: string[]): Record<string, string> {


Here you can return an array of tuples

nohehf · 2024-07-26T07:50:32Z

src/cli.ts

+const args = parse(Deno.args, {
+ boolean: ["help", "h"],
+ collect: ["H"],
+ string: ["Hapikey"],


I think the flag name is not really explicit, something like '--Global-Header', that may be shortened to '-GH', would be more explicit. Or maybe --AuthHeader?

sl4wa · 2024-07-27T08:23:39Z

Changes have been added, please review.

nohehf

Nice, maybe adding a -AH shorthand for the flag would be a nice addition

nohehf · 2024-07-29T07:26:57Z

src/index.ts

 const queryCollection = outrospectionToQueries(outrospection);
+ // Remove the authHeader from individual request headers
+ const headersWithoutAuth = headers.filter(([key]) => authHeader ? key !== authHeader[0] : true);


I don't think you need to filter it out now that it is passed separately

The purpose of this filtering is to avoid confusion. If an Auth Header is present, we should not include the same Auth Header at the request level. It should only be included at the Postman collection level.

nohehf · 2024-07-29T07:28:35Z

src/lib.ts

+ if (typeof header !== "string") {
+ throw new Error(`Header is not a string: ${header}`);
+ }
+ const [key, value] = header.split(": ", 2);


You need to trim both the key and value, as it was before

nohehf · 2024-07-29T10:02:06Z

src/lib.ts

 }

 async function query(
 url: string,
 query: string,
- headers?: Record<string, string>,
+ headers?: Array<[string, string]>,


this should not be optional

sl4wa · 2024-07-30T06:38:45Z

An alias could be only one single character, so added -A as an alias for AuthHeader.
Please review.

nohehf · 2024-08-26T09:04:59Z

Sorry I was of for about 3 weeks, and did not touch any computer
Getting back to work, let's merge this asap.

nohehf · 2024-08-26T09:06:11Z

src/cli.ts

+ : undefined;
+
+if (authHeader) {
+ headers.push(authHeader); // Add AuthHeader to headers for API access


my last concern is this part, I don't get why you add the authHeader to the regular headers. This defeats the point of having it separate no ?

it's basically needed to get a response :)
these are the headers which are being passed by the tool to the API, not the headers from the collection

nohehf · 2024-08-26T09:06:36Z

src/index.ts

 const queryCollection = outrospectionToQueries(outrospection);
+ // Remove the authHeader from individual request headers


And this should not be needed at all

nohehf · 2024-08-26T09:17:21Z

Hey @sl4wa, I just pushed a few changes to clean up some stuff, and simplify.
Let me know if this works for you, and i'll merge it once confirmed
Sorry again for the delay

sl4wa · 2024-08-26T10:04:18Z

Hi, i'm testing the changes. It does not work for me.

Seems that if we don't pass the new header along with the other HTTP headers it can't authorize.

nohehf · 2024-08-26T10:31:46Z

Seems that if we don't pass the new header along with the other HTTP headers it can't authorize.

Then what's the point of having it globally as a separate parameter, if we duplicate it everywhere? If it is in the right place in the collection it should work. I believe something is missing in the collection generation part. Maybe the requests need to specify that they rely on the global header ?

sl4wa · 2024-08-26T10:55:43Z

You're right, there is no need to duplicate the header on the each postman's request level. It should be specified only once on the collection level.

But we still need to add this header to the requests that are being passed by this tool to get the schema. Ater the latest changes I'm not able to do that.

Error fetching introspection query.
 Please verify your URL, authorization, and network connection.

Once rolling back to commit from Jul 31, all works fine.

nohehf · 2024-08-26T11:08:56Z

Once rolling back to commit from Jul 31, all works fine.

Yes but then it means that we duplicate the header, which defeats the hole point of this MR and can already be done via -H

Postman is currently having some issues: https://status.postman.com so I cannot test it.

If you can export a collection that has global authentication has you need it to json, I'll be happy to look into it, maybe:

    collection.auth = {
      type: "apikey",
      apikey: [
        { key: "in", value: "header", type: "string" },
        { key: "value", value: authHeader[1], type: "string" },
        { key: "key", value: authHeader[0], type: "string" },
      ],

is wrong.

nohehf · 2024-08-26T11:17:06Z

But we still need to add this header to the requests that are being passed by this tool to get the schema. Ater the latest changes I'm not able to do that.

Oh okay that's why you merged it.
I'm going to pass this to the introspection request, and you should be good.

nohehf · 2024-08-26T11:24:40Z

Passed the header to introspection too, without duplicating it everywhere

sl4wa · 2024-08-26T12:56:32Z

Passed the header to introspection too, without duplicating it everywhere

Thanks, it works good now!

With the latest changes the auth header is added to every Postman HTTP request. However I think we should filter it out and have it only at the collection level.

nohehf · 2024-08-26T14:01:17Z

With the latest changes the auth header is added to every Postman HTTP request.

Oh that's my bad then, the goal was precisely not to do this yes. Looking into it

nohehf · 2024-08-26T14:07:59Z

Fixed, was a little bug in my code (did not clone the header array so I was mutating it), should be all good

sl4wa · 2024-08-26T14:58:46Z

Thanks, it works perfectly fine now.

nohehf · 2024-08-26T15:02:31Z

merging :)

sl4wa · 2024-08-26T15:09:34Z

Thanks for approval.

I've noticed that in README we are specifying the v1.2.1 tag. This should probably be updated when you're ready.

sl4wa requested a review from nohehf as a code owner July 13, 2024 09:25

nohehf requested changes Jul 26, 2024

View reviewed changes

sl4wa requested a review from nohehf July 27, 2024 08:23

sl4wa force-pushed the add-apikey-auth branch from f09813c to 131cd32 Compare July 29, 2024 09:24

nohehf requested changes Jul 29, 2024

View reviewed changes

sl4wa requested a review from nohehf July 30, 2024 06:39

sl4wa force-pushed the add-apikey-auth branch 4 times, most recently from e99a717 to 5c87222 Compare July 31, 2024 13:11

Add support for Postman Authorization via API key

ad0517f

sl4wa force-pushed the add-apikey-auth branch from 5c87222 to ad0517f Compare July 31, 2024 13:13

nohehf reviewed Aug 26, 2024

View reviewed changes

fix: adjust auth header

9543478

fix: pass auth header to introspection

6ab9d7e

fix: allheaders

0422354

nohehf merged commit 2759def into Escape-Technologies:main Aug 26, 2024
2 checks passed

sl4wa deleted the add-apikey-auth branch August 27, 2024 07:02

		const queryCollection = outrospectionToQueries(outrospection);
		// Remove the authHeader from individual request headers

Add Support for Postman Authorization via API Key #52

Add Support for Postman Authorization via API Key #52

Conversation

sl4wa commented Jul 13, 2024 • edited Loading

Summary

Changes

Testing

Additional Notes

nohehf commented Jul 22, 2024

sl4wa commented Jul 22, 2024 • edited Loading

nohehf 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

Choose a reason for hiding this comment

sl4wa commented Jul 27, 2024

nohehf 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

sl4wa commented Jul 30, 2024

nohehf commented Aug 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nohehf commented Aug 26, 2024

sl4wa commented Aug 26, 2024

nohehf commented Aug 26, 2024

sl4wa commented Aug 26, 2024

nohehf commented Aug 26, 2024 • edited Loading

nohehf commented Aug 26, 2024

nohehf commented Aug 26, 2024

sl4wa commented Aug 26, 2024 • edited Loading

nohehf commented Aug 26, 2024

nohehf commented Aug 26, 2024

sl4wa commented Aug 26, 2024

nohehf commented Aug 26, 2024

sl4wa commented Aug 26, 2024

sl4wa commented Jul 13, 2024 •

edited

Loading

sl4wa commented Jul 22, 2024 •

edited

Loading

nohehf commented Aug 26, 2024 •

edited

Loading

sl4wa commented Aug 26, 2024 •

edited

Loading