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

Destination in server.rb logout now set and used correctly. #76

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Destination in server.rb logout now set and used correctly. #76

wants to merge 9 commits into from

Conversation

dyson
Copy link

@dyson dyson commented Jan 9, 2012

The destination URL does not makes sense to be a substitute to service URL if the service URL is not set. This commit fixes that behaviour.

@dyson
Copy link
Author

dyson commented Jan 9, 2012

Apologies the final block of ifs should needed an update to allow either forwarding to the service if it's a gateway (cas standard) or forward to the destination url if set (non cas standard but you already had implemented).

Also there is no logout template so if there is a continue_url passed you get an error. I set it to use the login template.

@zuk
Copy link
Member

zuk commented Feb 3, 2012

Potential changes to behaviour... needs closer evaluation before merging.

@luckyruby
Copy link

Any updates?

@ghost ghost assigned mitfik Dec 5, 2012
@mitfik
Copy link
Contributor

mitfik commented Dec 5, 2012

I work on that to clear this commit and prepare test for it.
Because in that form we can't merge it.

Thanks a lot for contribution.
Soon I will back to you.

@dyson dyson mentioned this pull request Dec 7, 2012
# no service is passed in the request, behaviour is per the CAS specification.

#default_service: 'https://au.edu.burnet.launchpad.dev/people/service'

Copy link
Contributor

Choose a reason for hiding this comment

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

I investigate that a little bit more and I honestly I do not see any reason why someone would like to use it.
Could you provide some scenario for default_service option?

Copy link
Author

Choose a reason for hiding this comment

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

This was just here because if someone goes straight to the rubycas server they just get a page saying they are logged in. I actually wanted them to go to something more useful like our main service. Also if they were already logged in and went straight to the rubycas server the same thing happens.

So if you had https://sso.example.com someone can go directly to that page and they get stuck.

Mos tof the time you want users to log in via a service such as https://service.example.com but they might not. This way you can default where they should go if they go straight to https://sso.example.com

I think as you point out below this was because continue_url was not coded correctly, and I can't remember what the specs say on that.

I've rolled my own solution since my initial playing around with rubycas-server so might be best to close this and submit a new cleaner patch. This patch was just a stepping stone that got it working for me how I needed it to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants