Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Move Active Authentication Mode into AutomaticAuthenticationHandler - #164

Closed
HaoK wants to merge 20 commits into
2-23-renamefrom
2-23-auto
Closed

Move Active Authentication Mode into AutomaticAuthenticationHandler#164
HaoK wants to merge 20 commits into
2-23-renamefrom
2-23-auto

Conversation

@HaoK

@HaoK HaoK commented Feb 24, 2015

Copy link
Copy Markdown
Member
  • Rename AuthenticationMode to AutomaticAuthentication bool on AutomaticAuthenticationOptions
  • Split old active behavior into a new base handler class : AutomaticAuthenticationHandler which derives from AuthenticationHandler<AutomaticAuthenticationOptions>
  • Remove SecurityHelper LookupChallenge and added two new virtuals to handle the old active authentication checks (which work when no scheme is specified)
  • Also attempted to address 401-403 with this change (needs more unit tests here)

cc @lodejard @Praburaj @blowdart @Tratcher

@ghost ghost added the cla-not-required label Feb 24, 2015
Tratcher
Tratcher reviewed Feb 25, 2015
View reviewed changes
Comment thread Security.sln Outdated

@Tratcher Tratcher Feb 25, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you didn't mean to check in this relative reference.

@HaoK HaoK Feb 25, 2015

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'll revert the global.json and revert all the sln changes as part of the final squash/merge, they won't make it back to dev

@HaoK

HaoK commented Feb 25, 2015

Copy link
Copy Markdown
Member Author

Updated PR, moved ShouldConvertChallengeToForbidden to AutomaticHandler, added a test for cookie/bearer to ensure they don't touch 401's if they weren't authenticated.

Tratcher
Tratcher reviewed Feb 25, 2015
View reviewed changes
Comment thread src/Microsoft.AspNet.Authentication/AuthenticationHandler.cs Outdated

@Tratcher Tratcher Feb 25, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't SignIn and SignOut also check for an AuthScheme match before doing anything here?

@HaoK HaoK Feb 25, 2015 via email

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HaoK HaoK Feb 28, 2015

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added tests verifying that exceptions are thrown (rather than sign in/sign out happening) when wrong auth type is specified

GrabYourPitchforks and others added 19 commits February 25, 2015 17:23
@HaoK
Fix and add tests for SignIn/SignOut
b97d0c3 
now correctly checks for auth scheme
@HaoK

HaoK commented Mar 2, 2015

Copy link
Copy Markdown
Member Author

Merged 775eb5e

@HaoK HaoK closed this Mar 2, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@HaoK @Tratcher @kevinchalet @GrabYourPitchforks