Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retrying Eve SSO (RetryingOAuth) using Refresh Token #116

Open
eaglerainbow opened this issue Dec 6, 2020 · 10 comments
Open

Retrying Eve SSO (RetryingOAuth) using Refresh Token #116

eaglerainbow opened this issue Dec 6, 2020 · 10 comments

Comments

@eaglerainbow
Copy link

eaglerainbow commented Dec 6, 2020

I have the situation that I am fetching an accessToken and a refreshToken through EveSSO. The point is that I have to persist the refreshToken (I am taking care of encryption at rest), because the authentication provided should be retained over multiple executions of the application.
As adviced on the main README.md, I was bootstrapping the ApiClient with

new ApiClientBuilder().refreshToken(refreshToken).clientID(clientid).build();

However, when performing the next call for which ESI requires authentication, I get back an unauthorized.
A little debugging showed me that only an instance of OAuth

grafik

and not an instance of RetryingOAuth is created. Apparently, this leads to the effect that the Authorization header is filled with a Bearer null value.
I also tried to manually configure the ApiClient instance manually, making sure that a constructor is being called which creates an Authentication of type RetryingOAuth. However, also there, no avail: I could not get it running. There, the Authorization header isn't filled at all - and a break point in method retryingIntercept of RetryingOAuth is never hit.

What am I doing wrong?

@eaglerainbow
Copy link
Author

eaglerainbow commented Dec 6, 2020

Also note the last sentence as of https://docs.esi.evetech.net/docs/sso/refreshing_access_tokens.html#sso-response:

Please note that the refresh_token returned may not be the same as the refresh token submitted. At some point in the future the EVE SSO will enable refresh token rotation for native applications. Make sure to update the refresh token stored on the client side in those cases.

As far as I can see, there is no logic in place, which would update the refresh token.

@GoldenGnu
Copy link
Collaborator

  1. If you're handling EveSSO yourself, you should not use ApiClientBuilder, but, instead use the refreshToken parameter on the endpoint functions
  2. ApiClientBuilder creates one instance of ApiClient with the refresh_token set, other instances will not have it set.
  3. ApiClient is thread safe, but, you need one instance per refresh_token if you're using the build-in auth. If you're using the refreshToken parameter that is not a problem.
  4. Auth is handled by the OAuth class (not the RetryingOAuth class)
  5. OAuth does indeed handle changing refresh tokens
    To get the updated refresh_token do:
final OAuth auth = (OAuth) client.getAuthentication("evesso");
auth.getRefreshToken()

I hope this is helpful, it's really hard to know what is wrong, when you're not showing your code.

@eaglerainbow
Copy link
Author

Thanks! I will check your suggestions and will come back.

@GoldenGnu
Copy link
Collaborator

GoldenGnu commented Dec 6, 2020

So, I made two mistakes:

refreshToken parameter on the endpoint functions

I think it's parameter is actually for the accessToken (not refreshToken)

If you're handling EveSSO yourself, you should not use ApiClientBuilder...

Actually, you always need to use ApiClientBuilder to set the user-agent. Just not the refreshToken

@eaglerainbow
Copy link
Author

I am trying to get all the implications of your statements, but I somehow fail to knot your statements together in my brain. Lemme go through your statements:

If you're handling EveSSO yourself, you should not use ApiClientBuilder, but, instead use the refreshToken parameter on the endpoint functions
I think it's parameter is actually for the accessToken (not refreshToken)

I think so, too: The token parameter is a replacement in case you cannot fill the Authorization header. I don't think that this is a major issue.

ApiClientBuilder creates one instance of ApiClient with the refresh_token set, other instances will not have it set.

Okay, that is feasible for me.

ApiClient is thread safe, but, you need one instance per refresh_token if you're using the build-in auth. If you're using the refreshToken parameter that is not a problem.

That is not the big problem: I have a single known user (whose username and password for login is statically known - if that would help). I need to regularly poll the content of an authentication-enabled endpoint. That user is specially set up for this, btw.
The problem there is that the accessToken expires latest after 20 minutes. Each time we get a new accessToken, the refreshToken might get updated as well. The old refreshToken becomes invalid. Also the refreshToken may have a finite validity (I did not understand the pattern yet, though).

Auth is handled by the OAuth class (not the RetryingOAuth class)

That is perhaps the thing which I don't understand properly:

accountData.setRefreshToken(result.getRefreshToken());

is the only place where the refreshToken is being adjusted (after running through an authentication flow). However, I don't see any chance that once accountData to invalidate it and have the refreshToken updated.

My understanding was that the RetryingOAuth class is there to ensure that a retry happens in case an authorization is rejected.
Apparently, I have too little knowledge/understanding about the various authentication methods the library provides.
Is there something like a documentation on how all those things are thought to work together?

OAuth does indeed handle changing refresh tokens
To get the updated refresh_token do:
auth.getRefreshToken()

Well, as far as I can see, that's the current refreshToken token:

return account.getRefreshToken();

which calls
public String getRefreshToken() {

That never updates the refreshToken.

I hope this is helpful, it's really hard to know what is wrong, when you're not showing your code.

Well, in case thanks for your assistance!
To make things easier, here is my use case again:

I need to call an authentication-enabled endpoint periodically. I have a special user set up which has the necessary authorities. The username and password of that user is known statically (i.e.through configuration at command line).
So far, I have followed the suggestion as outlined in

public static void main(final String... args) throws IOException, URISyntaxException, ApiException {
.
I fetched the refreshToken and created a new ApiClient instance using the builder:

		ApiClient rawApiClient = new ApiClientBuilder()
				.refreshToken(refreshToken)
				.accessToken(accessToken)
				.clientID(clientid)
				.okHttpClient(okHttpClient)
				.build();

As long as the accessToken is fine, everything works. If either the accessToken is expired or you leave off the accessToken (in the builder) and only provide the refreshToken, then there is no automatic attempt to retrieve a new access token.
Additionally, even the refreshToken sometimes seems to expire after some time; so the approach to update the refreshToken via a refresh_token Eve SSO call on demand also does not work in the long run.

The current approach that I am following now is the following:

  1. I have created a new class extending ApiClient. It delegates all calls to another "real ApiClient".
  2. Associated with each instance I have implemented an (inner class) Thread, which calls every 5 mins:
			OAuth authentication = (OAuth) this.delegate.getAuthentication("evesso");
			String refreshToken = authentication.getRefreshToken();

			OkHttpClient client = new OkHttpClient.Builder().build();
			OAuthClient oAuthClient = new OAuthClient(new OAuthOkHttpClient(client));
			TokenRequestBuilder tokenRequestBuilder = new OAuthClientRequest.TokenRequestBuilder(
					"https://login.eveonline.com/v2/oauth/token").setGrantType(GrantType.REFRESH_TOKEN)
							.setClientId(clientid).setClientSecret(clientsecret).setRefreshToken(refreshToken);

			OAuthJSONAccessTokenResponse accessTokenResponse = null;
			try {
				accessTokenResponse = oAuthClient.accessToken(tokenRequestBuilder.buildBodyMessage());
			} catch (OAuthSystemException e) {
				logger.error("Unable to retrieve new access token by refresh token", e);
				throw new ApiException(e);
			} catch (OAuthProblemException e) {
				logger.error("Error in oAuth infrastructure", e);
				throw new ApiException(e);
			}

			// ...

			final String newAccessToken = accessTokenResponse.getAccessToken();
			// warning: Also refreshToken must be replaced! See also last sentence of
			// https://docs.esi.evetech.net/docs/sso/refreshing_access_tokens.html#sso-response
			final String newRefreshToken = accessTokenResponse.getRefreshToken();

			authentication.setAuth(clientid, newRefreshToken);
			authentication.setAccessToken(newAccessToken);

			// ...

By this I enforce that the refreshToken gets updated periodically.

The problem at mine is:

  • My application may stop and I have to store the "current, valid refreshToken" for the restart.
  • The application may run significantly longer than the 20 minutes an accessToken is valid.

My gut-feeling currently tells me that a) this is way too complicated how I do it right now, and b) I don't really understand the Authentication interface approach the library uses. Unfortunately, I can't find any further documentation of the library with which I could try to educate myself 😞

@GoldenGnu
Copy link
Collaborator

Your problem is that you're mixing two systems.
You're doing OAuth yourself and also making eve-esi handle it.
You have to one or the other.

If you want to let eve-esi handle it:
-Do not do any OAuth stuff yourself, like updating the access token etc.
-Don't set the access token (You should not get an access token at all, eve-esi will handle that)
-Set the refresh token and client id
-Once your app closes down get the refresh token from OAuth and save it.

If you want to handle it yourself:
-Do all the OAuth stuff yourself, this include updating access tokens before they expire etc.
-Do not set access token, refresh token, or client id
-When using an authed endpoint, use access token parameter on the method.

That is it.

@GoldenGnu
Copy link
Collaborator

GoldenGnu commented Dec 8, 2020

Made a quick example you can try and run:

public static void walletUpdate(String clientId, String refreshToken) throws ApiException {
        //The ApiClient can be reused for this refreshToken and is thread safe
        ApiClient apiClient = new ApiClientBuilder().clientID(clientId).refreshToken(refreshToken).build();

        //Get the characterId
        final SsoApi api = new SsoApi(apiClient); //It's important to parse the ApiClient to SsoApi 
        CharacterInfo info = api.getCharacterInfo();
        int characterId = info.getCharacterID();

        WalletApi walletApi = new WalletApi(apiClient); //Again, parsing the ApiClient to WalletApi 

        //Update from ESI
        final Double walletBallance = walletApi.getCharactersCharacterIdWallet(characterId, "tranquility", null, null);
    }

@GoldenGnu
Copy link
Collaborator

It's also worth see the example of how to get an refresh token, if you want to completely switch to using eve-esi:
https://github.com/burberius/eve-esi/blob/master/src/test/java/net/troja/eve/esi/api/auth/SsoAuthTest.java#L182

@eaglerainbow
Copy link
Author

eaglerainbow commented Dec 8, 2020

Thanks for your suggestions! I have changed the logic to go along the lines of your proposal. Based on previous experience, I still have some doubts, but I want to give it an honest try. Over the night, a long-term standard-test is performed in the background - I would be more than happy, if that showed that my problem was solved with this.

@eaglerainbow
Copy link
Author

Your proposal turns out to be very robust (i.e. making eve-esi handling refresh management). The only thing which is very important is that you run at least one call to the token refresh endpoint of the EVESSO-generated refresh token. The refresh token provided there seems to be much more durable and sustainable than the "temporary" refresh token, the EVESSO generated initially.
I am happy with my solution here now. Thanks for your help!

As suggestion for improvement: If I hadn't ask, I would not have seen how I could have got to this information. I'd suggest to write this somewhere into some more detailed documentation (which I might have missed yet?!). This issue here could be used as a reminder/anchor for that. If you don't want to take it up, we can also close this issue here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants