Skip to content
This repository has been archived by the owner on Jul 31, 2024. It is now read-only.

GetLoginContext on UserInteractionService returns null when posting login #194

Closed
ajtowf opened this issue Aug 5, 2016 · 14 comments
Closed
Assignees
Milestone

Comments

@ajtowf
Copy link

ajtowf commented Aug 5, 2016

  1. RP redirects to IdSrv for signin with valid returnUrl
  2. IdSrv can retrieve information about RP like client name by getting LoginContext
  3. Post login
  4. Try to get the context (perhaps for logging something about the RP)
  5. Context is null

A workaround is of course to render the page with client info and then post it back, but it would be nice to be able to retrieve it from the context as when being redirected from RP.

Do I make any sense?

@ajtowf
Copy link
Author

ajtowf commented Aug 5, 2016

Never mind, I just ended up parsing the returnUrl with QueryHelpers to obtain client_id, maybe it's for the best.

@ajtowf ajtowf closed this as completed Aug 5, 2016
@leastprivilege
Copy link
Member

@brockallen do we need to look at that?

@brockallen
Copy link
Member

You need to round trip the returnUrl, and when you round trip it if it's not in the expected place then you can pass the value into the API.

@ajtowf
Copy link
Author

ajtowf commented Aug 24, 2016

Your answer doesn't make any sense, I think you've totally misunderstood what I was talking about. Never mind though.

@brockallen
Copy link
Member

brockallen commented Aug 24, 2016

If you invoke GetLoginContextAsync with a null param, then it looks in the expected query string for the value. If it's not there, then null will be returned. If on your login page you make postbacks and don't round-trip the value in the same query string, then where would GetLoginContextAsync look to find it? This is why on a post back you would need to round-trip the value, and if it's somewhere else (like a hidden form field) then you can pass it in explicitly to GetLoginContextAsync.

Or am I still misunderstanding?

@ajtowf
Copy link
Author

ajtowf commented Aug 24, 2016

That makes perfect sense now, thanks for clarifying that! I did roundtrip it, just didn't know I needed to pass it to GetLoginContextAsync. I'm guessing that method parses the query string as well. Anyways just passed returnUrl along and I got what I wanted. Thanks for having patience mate!

@brockallen
Copy link
Member

No problem -- and at some point this will be documented. If you have any other feedback on the IUserInteractionService, please let us know (in a new issue). Thanks.

@ajtowf
Copy link
Author

ajtowf commented Aug 24, 2016

What's best practice here; should I pass returnUrl to GetLoginContextAsync also when rendering the login page (HttpGet)? I don't need to do it, it still works, but what's your opinion?

@brockallen
Copy link
Member

Well, I'm open to suggestions. Since we need to allow it to be passed for the POST scenarios, we need to allow a param. But as a convenience since we know what the configured query string should be we can automatically look it up. Is the convenience behavior too confusing?

@ajtowf
Copy link
Author

ajtowf commented Aug 24, 2016

Well, it doesn't hurt to pass it in GET scenarios. The more I think about it, I prefer to be more explicit. Also I don't need to check if returnUrl is null, I can always go like:

var request = await _interaction.GetLoginContextAsync(returnUrl);
if (request != null) { // do stuff }

which simplifies it, instead of:

if (returnUrl != nulll) {
    var request = await _interaction.GetLoginContextAsync(returnUrl);
    if (request != null) { // do stuff }
}

do I make any sense?

@brockallen
Copy link
Member

Ok, that's good feedback. I'll keep it in mind for the RC. Thanks

@brockallen
Copy link
Member

// @leastprivilege can think about it too...

@brockallen
Copy link
Member

Ok, we consolidated the GetLoginContext and GetConsentContext and now the param is mandatory. Thanks.

@lock
Copy link

lock bot commented Jan 15, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants