ibm-apiconnect / devportal-addons

Example addon modules and themes for the API Connect Developer Portal
GNU General Public License v2.0
1 stars 2 forks source link

Cannot get access token with implicit flow #3

Closed alaincroisetiere closed 5 years ago

alaincroisetiere commented 5 years ago

Steps to reproduce :

  1. Install apiconnect-explorer 4.1.90
  2. Run dist/app/index.html
  3. Navigate to APIm Config APIs 2.0.0 / Get the Admin Settings
  4. Click on Try it and Get Token

A javascript error occur because tokenUrl is not present in OpenAPI v2.

image

redlanne commented 5 years ago

Using the latest code and that build I have been unable to reproduce, I do not get the error. I did get an error regarding a missing labelText, which I have now fixed. When clicking the GetToken button I get (correctly) redirected to a 404 message page on the swagger website. Console is clear.

Please bear in mind that the api's listed are just sample and there to test the rendering of various APIs, they are not fully functional APIs with real endpoints.

alaincroisetiere commented 5 years ago

Hi @redlanne, we have the same issue in production environment when API definition of version swagger 2.0 have only implicit flow in security definition.

TestToolAccessToken.js

 async getToken() {
    // ...

    // mixed content check - i.e. calling http from https - not allowed.
    if (window.location.protocol === 'https:' && this.state.flags.oauthTokenUrl.lastIndexOf(window.location.protocol) !== 0) {
      newState.authError = getMixedContentMessage(t);
      this.setState(newState);
      return;
    }

    // ...
  };

this.state.flags.oauthTokenUrl is null because we cannot define tokenUrl in a security definition of type oauth2 with implicit flow.

The same issue occur with the POST /pet API of petstore definition.

redlanne commented 5 years ago

ah yes - I got sidetracked by the console errors you were talking about - this is a known limitation as documented in the KC for APIConnect

https://www.ibm.com/support/knowledgecenter/en/SSMNED_2018/com.ibm.apic.devportal.doc/cmsportal_testtool.html

https://www.ibm.com/support/knowledgecenter/SSMNED_5.0.0/com.ibm.apic.devportal.doc/cmsportal_testtool.html

The explorer has recently i.e. from version 4.1.90 had another option added which makes this clearer. @chrisdudley do you want to make changes to the v5 add-on module that uses this?

alaincroisetiere commented 5 years ago

JavaScript error occurred when try to get access token on POST /pet with the configuration below. If you uncomment GET /path_added_to_fix_get_pet, there is no error when you click on Get token but i don't want to add an undisired path to fix it.

window.apiConnectExplorer = {
   attachPoint: '#root',
   apis: [
      {
         "swagger": "2.0",
         "info": {
            "version": "1.0.0",
            "title": "Swagger Petstore"
         },
         "host": "petstore.swagger.io",
         "basePath": "/v2",
         "schemes": ["https"],
         "paths": {
            //"/path_added_to_fix_get_pet": {
            //   "get": {
            //      "operationId": "path_added_to_fix_get_pet",
            //      "consumes": ["application/json", "application/xml"],
            //      "produces": ["application/xml", "application/json"],
            //      "parameters": [
            //         {
            //            "in": "body",
            //            "name": "body",
            //            "schema": {}
            //         }
            //      ],
            //      "responses": { "200": { "description": "" } },
            //      "security": [{ "petstore_auth_app": ["write:pets"] }, { "petstore_auth_implicit": ["write:pets"] }]
            //   }
            //},
            "/pet": {
               "post": {
                  "operationId": "addPet",
                  "consumes": ["application/json", "application/xml"],
                  "produces": ["application/xml", "application/json"],
                  "parameters": [
                     {
                        "in": "body",
                        "name": "body",
                        "schema": {}
                     }
                  ],
                  "responses": { "200": { "description": "" } },
                  "security": [{ "petstore_auth_implicit": ["write:pets"] }]
               }
            },
         },
         "securityDefinitions": {
            "petstore_auth_implicit": {
               "type": "oauth2",
               "authorizationUrl": "https://petstore.swagger.io/oauth/authorize",
               "flow": "implicit",
               "scopes": {
                  "write:pets": "modify pets in your account"
               }
            },
            "petstore_auth_app": {
               "type": "oauth2",
               "tokenUrl": "https://petstore.swagger.io/oauth/token",
               "flow": "application",
               "scopes": {
                  "write:pets": "modify pets in your account"
               }
            }
         },
         "definitions": {
         }
      }
   ]
};
chrisdudley commented 5 years ago

Implicit isn’t likely to work, from an api connect perspective datapower would only return a token to the redirect uri defined in an app and explorer doesn’t have an endpoint to receive it. As such we’ve changed the ux in the latest code when running in our portal to make it clear you need to source the token yourself in that flow.

If you are using explorer outside ibm api connect and think that flow could work for you then a PR is welcome, but I think this is now resolved from our point of view.

alaincroisetiere commented 5 years ago

Hi @chrisdudley, At the moment API Connect does not return the token to the redirect uri because javascript error occur before in apiconnect-explorer@4.1.90 npm package. Where could I do a PR on that package ?

chrisdudley commented 5 years ago

It won’t. Sorry but I’m having trouble seeing the point in what you’re attempting. You want to trigger apic to send a token to the redirect uri registered for that application from the test tool? The user would have to then go dig it out of random logs for wherever they have registered for their application. Not very nice usability which is why we’ve intentionally not done that.

Instead it now has a text box to paste the token in and you need to get the token yourself.

The explorer source isn’t on github now I think about it but obviously you can get it from npm so feel free to attach patches to issues here.

alaincroisetiere commented 5 years ago

You want to trigger apic to send a token to the redirect uri registered for that application from the test tool?

Yes, instead of JavaScript error. Developer user can use redirect uri like https://localhost?#access_token=ey.... or https://jwt.io/ and grab the token, past it and try api. You are right, this not perfect but better then JavaScript error.

chrisdudley commented 5 years ago

I understand, not really a use case we had in mind, but if you can find a patch that works we'll happily merge it in :-)

alaincroisetiere commented 5 years ago

Hi @chrisdudley, only test if this.state.flags.oauthTokenUrl is null before using it in getToken method of TestToolAccessToken.js. When API definition have only implicit flow in security definition this property is not set. Thank you!

TestToolAccessToken.js

 async getToken() {
    // ...

    // mixed content check - i.e. calling http from https - not allowed.
    if (window.location.protocol === 'https:' && !this.state.flags.oauthTokenUrl && this.state.flags.oauthTokenUrl.lastIndexOf(window.location.protocol) !== 0) {
      newState.authError = getMixedContentMessage(t);
      this.setState(newState);
      return;
    }

    // ...
  };
chrisdudley commented 5 years ago

Am I missing something there, but your check would only pass if it isnt set and it would then fail on the next step where it checks if the value it now knows isnt set starts with the window protocol ?

The ! seems wrong to me ?

Or if you really do mean ! then it should be || not && and group the second 2 checks ?

chrisdudley commented 5 years ago

I've changed it to be

    if (window.location.protocol === 'https:' && this.state.flags.oauthTokenUrl && this.state.flags.oauthTokenUrl.lastIndexOf(window.location.protocol) !== 0) {
      newState.authError = getMixedContentMessage(t);
      this.setState(newState);
      return;
    }

Which should at least avoid any javascript errors.

alaincroisetiere commented 5 years ago

It works fine @chrisdudley! Thank you very much.