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

Adding an opt-in policy for redirecting if we already have a logged-in Principal. #32

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stevematney
Copy link

No description provided.

@jeremydmiller
Copy link
Contributor

@stevematney,

I think you need to create an all new abstraction for the redirecting on being logged in. IAuthenticationRedirect is semantically a different concept altogether.

LoggedInUserRedirector -- you can't make the test by checking for the presence of an IPrincipal. Can you use IAuthenticationService instead here? There's a couple scenarios where IIS will stick a principal on that isn't ours and we might choose to use some kind of claims based authentication in the future and checking for the principal won't work in those scenarios.

LoggedInUserRedirectorTester -- do you know about InteractionContext? Coulda saved some code there.

Might be easier to use the [Filter(typeof( LoggedInUserRedirector))] attribute on the action for logging in. Right now, that policy isn't applied at all. Is that what you meant to do? My vote would be to go ahead and apply the filter, but register a nullo.

I'd really like to see an integration test for this behavior first.

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

Successfully merging this pull request may close these issues.

2 participants