Skip to content

[Test branch] Logout#1159

Closed
corneadoug wants to merge 1 commit intoapache:masterfrom
corneadoug:test/logout
Closed

[Test branch] Logout#1159
corneadoug wants to merge 1 commit intoapache:masterfrom
corneadoug:test/logout

Conversation

@corneadoug
Copy link
Contributor

What is this PR for?

This is a test branch related to #1140
This won't be merged

What type of PR is it?

Bug Fix

How should this be tested?

Login and Logout

Screenshots (if appropriate)

Firefox - Grunt Serve
firefox-serve

Firefox - Grunt Build
firefox-classic

Chrome - Grunt Serve
chrome-serve

Chrome - Grunt Build
chrome-classic

Safari - Grunt Serve
safari-serve

Safari - Grunt Build
safari-classic

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@prabhjyotsingh
Copy link
Contributor

This works perfectly with /** = authc, but #1140 was for fixing /** = authcBasic

@corneadoug
Copy link
Contributor Author

@prabhjyotsingh Oh, that's tricky, my initial comment was about authc.

The fun thing is that authcBasic isn't even in the doc, nor as choice in the shiro.ini anymore. Do we still need it? Even pressing cancel on that alert send you to the authc home page.

After testing them all here is the result:

#1140 - Both authc and authcBasic works
Master - Both authc grunt serve and authcBasic grunt serve do not work
This PR - authcBasic and authcBasic grunt serve do not work

@prabhjyotsingh
Copy link
Contributor

prabhjyotsingh commented Jul 11, 2016

Agreed @corneadoug. Though we don't have authcBasic in our doc, but whoever knows or had used shiro in past can choose to use it, and IMO whatever features shiro provides we should be able to leverage them as well.

@corneadoug
Copy link
Contributor Author

@prabhjyotsingh If we can remove it, then maybe it would be nice to do so in the future.
Especially since it makes the code more complicated.
For now, let's go with #1140

@prabhjyotsingh
Copy link
Contributor

I don't have a strong view on any, fine either way. Just that I thought authcBasic should work.

@corneadoug corneadoug closed this Aug 8, 2016
@corneadoug corneadoug deleted the test/logout branch August 8, 2016 10:36
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