I’m testing an upgrade to 4.0.46 and the linkedin provider seems to fail with “error=invalid_request&error_description=You+need+to+pass+the+“state”+parameter”.
This seems to be related to Add workaround for DNOA OAuth2 error , where an IClientAuthorizationTracker is inserted, and so the state query string parameter doesn’t get inserted into the oauth request.
If we restore DNOA’s default behaviour, on PrepareRequestUserAuthorization it tries to set a DotNetOpenAuth.WebServerClient.XSRF-Session cookie, which it later checks on ProcessUserAuthorization. But this cookie, which exists on the OAuthProvider, disappears somewhere on the stack. If it gets properly propagated up to the response sent to the browser, then the DotNetOpenAuthTokenManager might not be needed.
A quick workaround would be to insert a state parameter on the linkedin request and ignore it later, but I think this might open up a security hole, since that state parameter is used to avoid csrf vulnerabilities.
@mythz, do you have a clue as to why that cookie on the http result returned from OAuth2Provider might not be sent back to the client?
It copies the OutgoingWebResponse from DNOA and copies the HTTP Headers and Cookies into a HTTP Result which is what adds it to the current response.
Having a closer look it appears that the Set-Cookie HTTP Header is being suppressed in my local ASP.NET Host, so I’ve added a new Cookies collection to IHttpResult in this commit so later when the HttpResult response is written it calls the explicit SetCookie API’s on the underlying HttpResponse, this should now solve the previous workaround as well - thx for pointing it out.
This change is now available from v4.0.47 that’s now available on MyGet.
I might be having a small attack of the dumbs, but on https://www.myget.org/F/servicestack/Packages the only package I see is for ServiceStack.Authentication.MongoDB. Am I missing something?
Great, thanks. I’ve tested and it works now. We’ll wait for a final version of .47 before using it in prod.
As a side note, could the myget packages have a build number (4.0.47-beta1231) attached to them, and maintain the regular versioning on nuget packages (4.0.47)?
That way I could update our software to the current “beta” version, and then upgrade again when the final version is released, and not have to clean the nuget cache due to packages having the same version.
Sorry no, the odd/even number convention for disambiguating pre-release vs release builds is widely used and provides a good indication if its an official or pre-release package. To simplify visibility and reduce confusion we also don’t pollute the NuGet feeds with anything other than official packages. The current setup of pre-release packages on MyGet and official release packages on NuGet is easy to understand and manage from the top-level NuGet package feeds.
No, sorry, I wasn’t suggesting polluting nuget with the pre-relase packages! And I didn’t catch on the odd/even number convention, so that’s cool.
The core of my suggestion was to include a build number, if possible, on the pre-release packages, to avoid having to clear the nuget cache. But it might be cumbersome to update all projects if you’re iterating fast and there’s a lot of nuget dependencies between projects. I’m not sure if it’s possible to depend on 4.0.47-* on the current nuget toolchain (as it is on the project.json toolchain).
ok cool, but not looking to add modify the packages to include build numbers either, the NuGet packages on MyGet are exactly the same as the NuGet packages they’re just published to MyGet instead of NuGet so I don’t want to change how the packages are created or what dependency versions are referenced.
Updating MyGet packages is even easier than updating NuGet as it doesn’t need an explicit Upgrade, I just delete the /packages folder then open the solution and a do a full Rebuild to get NuGet to download the latest packages from MyGet.
Ok. I might have mentioned that in my scenario we have a teamcity build configuration compiling the project, and so if we push a branch with myget packages we might also need to flush the nuget cache on that remote machine, which is a bit more cumbersome. We also don’t want to always flush that cache to avoid delaying builds.
But since this is just for odd (unstable) versions, I guess it’s workable, since we’re more concerned with versions for production usage.
Just to check, this fix would exist on release 4.0.48, since 47 is the unstable correct?
