swagger-api / swagger-codegen

swagger-codegen contains a template-driven engine to generate documentation, API clients and server stubs in different languages by parsing your OpenAPI / Swagger definition.
http://swagger.io
Apache License 2.0
16.8k stars 6.02k forks source link

[C#]Would be nice if code generated adds implementation for handling refresh tokens #5848

Open HappyTreesDev opened 7 years ago

HappyTreesDev commented 7 years ago
Description

I think it would be nice if there was some sort of implementation in the generated code that handles Oauth2 Refresh Tokens. I am thinking there would be a configuration option that enables auto refresh, and the generated code would then check the expires time of the token and automatically call to refresh the auth token if it has expired and the configuration flag is set. Currently, I have two options to do this how the code is generated itself. I either have to add the code into the project before making the dll, or I have to manually check this info on the consumption side of things.

Swagger-codegen version

Master

wing328 commented 7 years ago

@HappyTreesDev thanks for the suggestion. May I know if you've time to contribute the enhancement?

I can work with you and show you some good starting point to implement the feature.

HappyTreesDev commented 7 years ago

I do not have the most time right now, but maybe in a month or so I will have some free time.

wing328 commented 7 years ago

@HappyTreesDev no problem. Ping us when you are available to contribute the enhancement.

jimschubert commented 7 years ago

@HappyTreesDev do you have an example of how you've modified generated code? It could potentially be an easy pickup.

HappyTreesDev commented 7 years ago

@jimschubert I do not have a good example. The modified code I made is not elegant in any way and involves being tightly bound to my xamarin pcl. I think it is not the right approach for implementing the solution, but as I think more I will figure out a way to update it.

Bogdans-Sahajevs commented 6 years ago

Hello all. Any development on this topic? If no one took it up, I'd like to try myself at extending swashbuckle/swagger but don't know where to start.

I imagine AspNetCore would involve app.UseSwaggerUI() { c.InjectOnCompleteJavaScript(" my JS that manages and refreshes OAuth tokens ") }

Bogdans-Sahajevs commented 6 years ago

Alright, so wrote something. See attached file OAuthRenew.JS.txt

In your app.UseSwaggerUI(c =>...); Load a script like this c.InjectOnCompleteJavaScript("/Content/Swagger/OAuthRenew.js");

Make sure you actually place "OAuthRenew.js" into "/Content/Swagger/" and your app can load static files (may need to call app.UseStaticFiles(); from your Startup.Configure)

jimschubert commented 6 years ago

@Bogdans-Sahajevs I'm not sure I understand the need to auto renew an Oauth token from Swagger UI.

I'd be reluctant to include this behavior in the Asp.net Core generator because a refresh token is almost like a password, and requires some additional security considerations. For instance, if someone was testing an API in Chrome over http (cuz we all do that locally), and accessing a third party API over https, storing the refresh token on the global window would allow any chrome extension with content scripts to have access to your secret refresh token. This is bad because refresh tokens are long lived.

I think the implementation could be submitted to swagger-ui to open it up for discussion. In my opinion, I'd recommend: only refreshing with this script if the current page is https, and supporting per-path object auth settings. Swagger allows different auth for each path and each operation, it also allows individual paths to remove the global security settings for that path (see spec). The reason I suggest opening a PR or Issue with the swagger-ui repo is because I recall being presented with OAuth dialog when my token expired in the past... but I can recall if I had a refresh token.

Bogdans-Sahajevs commented 6 years ago

@jimschubert You're right that storing refresh_token (in JavaScript variables accessible via browser dev. tools) isn't the best. In fact I myself wouldn't recommend including it, and wasn't intended as such. It's just something that works for me for now. I've never contributed to an open-source project, don't even know where to start

There are bigger problems in the current "Swashbuckle.AspNetCore-master" which I downloaded from "https://github.com/domaindrivendev/Swashbuckle.AspNetCore" on 08/30/2017, which I used for reference : Obtaining "access_token" based on "authorization_code" is done on Client Side with JavaScript, which means these can be seen with browser DevTools or Fiddler:

It's client secret for crying out loud!!! The whole implementation has to be changed to only doing that on server side.

I'll look into creating an actual "commit" to this project once I have more time

jimschubert commented 6 years ago

@Bogdans-Sahajevs

There's no issue with a developer using a developer tool like Swagger UI and issuing these from the client. OAuth should only be initiated over SSL. If the OAuth provider is accessible over HTTP only, you have a lot more than just your client that can view the request packets. Similarly, there's no real concern of a developer viewing secure requests they're making.

Swagger UI isn't intended to be a production bit of code, rather it is a tool for documentation and evaluation of an API. APIs do document public APIs using Swagger UI, but I don't know any that expect it to be the main API entry.

That said, you'll also need to understand OAuth a bit more to understand what needs to be secured and where to do it. I'm not saying you don't, just in general people need it. A client id is used to identify an application as an accessor to an API. A client secret is a secret between the client and the API. Depending on how you consume OAuth, you can do this either on the client or the server. That is, if your backend is the "client" to an external auth store (external API) and this is hidden implementation from your consumers, then yeah you want that hidden. If your API uses something like Facebook to authenticate, then you've set up a trust relationship with Facebook and your client (whether Swagger UI, a mobile app, or some other client), then your client needs the id and secret. But, that doesn't mean you want to transfer your personal id/secret to a client.

If, rather than a third party OAuth, you've implemented your own provider, then your token endpoint will require the client id and secret. These would be values you've generated and provided to your consumers (your clients) and they would use these to initiate the OAuth workflow to authenticate with your API.

I hope that answers any questions you may have about OAuth. If not, Digital Ocean has a really good intro article here: https://www.digitalocean.com/community/tutorials/an-introduction-to-oauth-2


OAuth protocol aside, Swashbuckle isn't affiliated as far as I know with swagger-codegen. I believe that project's maintainer has contributed to the OpenAPI specification, which is the community driven project for driving the specification previously called "Swagger Specification". That project also only embeds Swagger UI and isn't affiliated with that project. The maintainer may contribute to Swagger UI, I don't know.

There's also a different group of developers that maintain Swagger Codegen (this project) than maintains Swagger UI (https://github.com/swagger-api/swagger-ui). There are probably only a few engineers who are intimately familiar with both projects. I've looked through Swagger UI code pretty in depth, but it's probably been more than a year since I've really looked at it. For that reason, it's usually best to submit issues to a more appropriate repository.

Usually, if you find a well defined bug and submit to a repo that a contributor thinks is better suited elsewhere, a contributor usually opens the issue there and links back to the original. That's not usually done for questions or concerns.

I hope that all helps. If you have more questions or whatever, feel free to open an issue with the question.

jimschubert commented 6 years ago

Haven't heard back, I'm not sure if what I said was clear or not.

You may also want to check out this possible related issue: swagger-api/swagger-ui#2837