OfficeDev / orc-for-java

Office REST Client (ORC) for Java
MIT License
5 stars 3 forks source link

Handling access token expiry #5

Open MikeN123 opened 9 years ago

MikeN123 commented 9 years ago

The default JavaDependencyResolver expects an OAuth access token as String. I think there are a couple of issues with this approach when it comes to token renewal. Normally, you want to reuse the dependency resolver (it can contain your HttpClient state and it is unnecessary to recreate it each time), but if the access token is a string there is no way to do proper token renewal.

Ideally, the dependency resolver would take some other object, and use that to retrieve its access token. On an HTTP 401, it would signal this object that the access token has expired, and optionally retry the request.

In the current architecture, using some factory object to provide the access token is possible by providing a custom dependency resolver. But handling an HTTP 401 error in a nice way is impossible - the error will always bubble up through the client libraries and you have to handle it at each place where you perform a REST API call.

marcote commented 9 years ago

@MikeN123 We've been discussing this issue before internally. As you mention, there is no clear way to handle a 401 error and what's worse, I'm not sure if we can do much about it. OAuth is being handle by another library, ADAL https://github.com/AzureAD/azure-activedirectory-library-for-java and what they offer to handle this scenario is not good enough IMO. If you think you came up with an alternative approach I can forward the feedback to them. In case we can do something from the SDK's / ORC side, let me know and I will do what I can.

MikeN123 commented 9 years ago

I disagree ADAL is the problem here. Of course the integration between ADAL and ORC could be better, but if we have to write our own access code retrieval logic anyway. It would still be possible to:

With these small changes, it would at least be possible to provide a custom DependencyResolver which maintains a cache for the access token and fetches a new token on failure.

marcote commented 9 years ago

I see. Let me see what I can do.