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

Using TokenProvider.Token from within a test context causes deadlock #84

Open
HenryKeen opened this issue Nov 4, 2019 · 9 comments
Open

Comments

@HenryKeen
Copy link

Hi 👋 Just a quick one...

I'm trying to get an anonymous token to pass back to the user from within an API. I am setting everything up as expected in the readme and realised that I can actually resolve the collection of TokenProviders directly in order to get an anonymous token.

My source code...

public AnonymousTokenProvider(IEnumerable<ITokenProvider> tokenProviders)
{
    _tokenProvider = tokenProviders.First(tp => tp.TokenFlow == TokenFlow.AnonymousSession);
}

public Task<Token> GetToken()
{
    var token = _tokenProvider.Token;
    return Task.FromResult(token);
}

I have an integration test that invokes this class which works fine when it runs on its own but fails when it runs with other tests in parallel. I think we're hitting an async/await deadlocking issue here because _tokenProvider.Token is invoking an async call and calling .Result on the task.

I'm fairly certain this is the cause of my issue because when I inline the code from this repo but make it async all the way it solves my locking issue.

Is there a reason why this can't be replaced with an async method call instead? Would anyone mind if I submit a PR to fix this?

Thanks! 👍

@jenschude
Copy link
Contributor

jenschude commented Nov 4, 2019

As the Token is needed before executing a request and the auth providers are delegating handlers the retrieval should be blocking or at least locking else it would issue multiple parallel auth requests

@HenryKeen
Copy link
Author

Hi @jenschude thanks for such a speedy reply!

I understand, thats a very good reason! Would you be open to wrapping this code this.GetTokenAsync(this.GetRequestMessage()) in a public method named something like GetTokenAsync() and adding it to the interface so that we can call it explicitly?

@jenschude
Copy link
Contributor

Another possibility would be to make a call against the project endpoint for example at application start or in your test fixture which creates the container.

Also our experience was that we had to disable the parallelism in xUnit tests especially for CI environments which typically only have 1 vCPU and xUnit defaults to CPU number as maximum threads which will easily result in deadlocks with asynchronous code.

@HenryKeen
Copy link
Author

Thanks @jenschude! On this occasion I need to guarantee that each token is unique as we are issuing it at the start of user journeys on our site. Wouldn't getting the token by calling the project endpoint mean that tokens are cached for the lifetime of the Client? Is there a way I can ensure that it is never cached?

@jenschude
Copy link
Contributor

jenschude commented Nov 5, 2019

Could you give a more detailled description on your use case? Cause the ClientCredentials flow is not meant to issue a token per user but per node.

Also you could not limit the API to a specific customer using a token issued with client credentials flow.

For this use case there is the PasswordFlow, AnonymousTokenFlow and RefreshTokenFlow. The SDK supports these flows to be used in a mobile app. For server side there has to be a new TokenProvider implemented.

Please also take a look at https://docs.commercetools.com/http-api-authorization

@HenryKeen
Copy link
Author

HenryKeen commented Nov 5, 2019

Sorry for the confusion. It is the anonymous token flow that we're after. The only thing I would like to do with the client is to get a brand new anonymous token every time I invoke it. I don't want to use the client to get the data or anything else - this happens in other components that will use the access token that I'm returning to the user here.

The reason for this is that we need to return the access token in a Set-Cookie header from the website when a user doesn't have one.

So the user will make a request to the website, the website code makes a server-side call to get a new anonymous token (using this client ideally), and returns it in a Set-Cookie header.

@jenschude
Copy link
Contributor

This is not what the actual implementation of the AnonymousFlow would support. As already said the implementation could be used in a mobile app which doesn't have different user contexts.

For supporting your scenario you would need a different implementation which the SDK is lacking atm

@HenryKeen
Copy link
Author

Okay thanks @jenschude

@andersekdahl
Copy link

Is there any docs or code to read for setting this up in a server side app?

From looking at the code we could solve it by overriding the credentials store and also picking different clients (with different token flow) depending on the current http context.

Does that sound feasible or am I missing something?

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

3 participants