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.

Sign up for GitHub

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

Jump to bottom

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
+17 −5
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d5ea7c0
Destination in server.rb logout now set and used correctly.
Jan 5, 2012
8e50a1a
Added gateway service redirect and changed logout template.
Jan 9, 2012
1a0b306
Fixed syntax.
Jan 11, 2012
bc9c7b5
Fixed syntax again.
Jan 11, 2012
4e20749
Added default service to config.
Jan 15, 2012
901e937
Added default service to login post request.
Jan 18, 2012
37c14b6
Updated logout destination logic.
Feb 22, 2012
de81060
Merge branch 'master' of https://github.com/rubycas/rubycas-server in…
Feb 24, 2012
a0cea4a
Updated exmaple config to include default service.
Mar 5, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions config/config.example.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,15 @@ log:
#enable_single_sign_out: true


##### SERVICES #################################################################

# The default service is used when no service is passed in the request

# This is not part of the CAS specification. If no defualt service is set, and
# 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.

##### OTHER ####################################################################

# You can set various ticket expiry times (specify the value in seconds).
Expand Down
13 changes: 8 additions & 5 deletions lib/casserver/server.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ def self.init_database!
headers['Expires'] = (Time.now - 1.year).rfc2822

# optional params
@service = clean_service_url(params['service'])
@service = clean_service_url(params['service']) || settings.config[:default_service]
@renew = params['renew']
@gateway = params['gateway'] == 'true' || params['gateway'] == '1'

Expand Down Expand Up @@ -400,7 +400,7 @@ def self.init_database!
Utils::log_controller_action(self.class, params)

# 2.2.1 (optional)
@service = clean_service_url(params['service'])
@service = clean_service_url(params['service']) || settings.config[:default_service]

# 2.2.2 (required)
@username = params['username']
Expand Down Expand Up @@ -516,7 +516,8 @@ def self.init_database!
# "logout" page, we take the user back to the login page with a "you have been logged out"
# message, allowing for an opportunity to immediately log back in. This makes it
# easier for the user to log out and log in as someone else.
@service = clean_service_url(params['service'] || params['destination'])
@service = clean_service_url(params['service'])
@destination = params['destination']
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to change that a little bit.
Because right now we have tree very similar things.:

service - url for "website" from where user came
destination - url for "website" where user should have possibility to logs in 
continue_url - url for "website" where user should have possibility to be redirected.

From which:

  • continue_url is not used anywhere because even if you will render login page instead of missing logout, there will be no any information with it.
  • destination in form which you provide is not used anywhere because there is no any handle for that in the form.

So I doubt that we need all of them. I think that we should use only service url which can be used as it is right now, plus possibility to do not redirect user after logout but just render login page with this service_url

For example:

/logout?service=http://service1&r=f <- this one will render login page (f= false, t= true, r=redirect)
/logout?service=http://service2&r=t  <- this one will redirect to service

and we could add settings for it, like:

redirect_to_service_after_logout: false

so everybody will have possibility to set default behavior and in case if it will be needed, we can force different behavior by passing param into url &r=t or &r=f

What do you think about that?

@continue_url = params['url']

@gateway = params['gateway'] == 'true' || params['gateway'] == '1'
Expand Down Expand Up @@ -559,10 +560,12 @@ def self.init_database!

@lt = generate_login_ticket

if @gateway && @service
if @gateway && @destination
redirect @destination, 303
elsif @gateway && @service
redirect @service, 303
elsif @continue_url
render @template_engine, :logout
render @template_engine, :login
else
render @template_engine, :login
end
Expand Down