Escape-Technologies / graphman

Quikly scaffold a postman collection for a GraphQL API. Compatible with Postman & Insomnia.
MIT License
240 stars 11 forks source link

Add Support for Postman Authorization via API Key #52

Closed sl4wa closed 2 weeks ago

sl4wa commented 2 months ago

Summary

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

Changes

Testing

  1. Run the CLI with the new --AuthHeader (-A) parameter.
  2. 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 commented 1 month ago

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 commented 1 month ago

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.

image

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

sl4wa commented 1 month ago

Changes have been added, please review.

sl4wa commented 1 month ago

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

nohehf commented 2 weeks ago

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

nohehf commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

Passed the header to introspection too, without duplicating it everywhere

sl4wa commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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

sl4wa commented 2 weeks ago

Thanks, it works perfectly fine now.

nohehf commented 2 weeks ago

merging :)

sl4wa commented 2 weeks ago

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.