Skip to content
This repository has been archived by the owner on Jul 24, 2020. It is now read-only.

Non-CAS authentication #2

Closed
31 tasks done
caseywatts opened this issue May 29, 2012 · 42 comments
Closed
31 tasks done

Non-CAS authentication #2

caseywatts opened this issue May 29, 2012 · 42 comments

Comments

@caseywatts
Copy link
Collaborator

2014-09-28

For now, we're going to implement Devise authentication with the devise_cas_authenticatable gem for CAS. We will also try to write up example migrations and instructions for alternative Devise authentication schemes.

To Do:

  • Add devise gemlater
  • Add devise_cas_authenticatable gem
  • Set up devise authentication before_filters instead of old RubyCAS filters
  • Write migration to change login column name to username (delete current migration)
  • Replace all uses of login for ActiveRecord lookups with username
  • Remove custom current_user method in lieu of built-in Devise method

2014-10-12

Most / all of the above was completed today and almost all of the current test suite is passing. My thoughts are outlined in the comment below; I'll try to turn them into a proper to-do list soon.

2014-10-13

All tests now pass. I tried getting database_authenticatable working and it was messy, but I think providing example code at least for that case is important. If I'm strugging to get this working, so will future clients. I'd like to see if we can do the following:

  • remove all CAS workflow dependencies make sure both authentication methods work
    • redirect hackery
    • user new hackery
    • any other hackery :-P
  • get database_authenticatable working, using our current user resource
    • add required parameters to strong params (commented out by default)
    • make sure edit and update work
    • set up proper Devise configuration in devise.rb, with options for both username or email login
    • set up required database migrations (with .example file extension so they don't run by default)
    • COMMENT ALL THE THINGS
  • write tests for authentication (I'm not sure where these go... maybe just in the controller specs, one context per controller?)
  • think about implementing other devise modules
    • recoverable
    • trackable
  • clean up devise.rb with only required / related options and comment where necessary
  • document the procedure for enabling database_authenticatable either in the README or wiki or both
  • look into making database_authenticatable the default and switching to CAS using the deploy script / variable (related to Document and set up generalized deployment #683)

2014-10-19

  • look into implementing email LDAP lookup for database authenticatable

2014-10-25

  • manually test :recoverable
    • look at generated devise views to figure out how to set up the UI elements
  • write integration tests for sign up (using password authentication)
  • write integration tests for resetting password (obviously using password authentication)
  • go through devise.rb again and take out settings that shouldn't be changed :-)
  • document the process for enabling CAS authentication in the README
  • document our authentication workflow in the wiki
  • look into using LDAP lookup via e-mail for password authentication
@ghost ghost assigned iansuvak Jun 22, 2012
@caseywatts
Copy link
Collaborator Author

I'm a big fan of using external authentication like omniauth for a google account, etc
Let's try one of those? :)

@iansuvak
Copy link

I've implemented omniauth version of cas.. Adam and I decided that it would be too much to enable both simultaneously so we will make it a setting only settable at install time. Stalling until @maltyeva is done with the app settings refactoring

@iansuvak
Copy link

I am done for the summer but here is the summary fo the current status of the Devise branch and possible problems...

The CAS server host adress is setup in devise initializer as CAS_HOST variable. That is the only bit of code setup necessary for this.

I have implemented an extra step in the application setup where the Authorization settings and the LDAP settings are configured. The Devise works as following: It uses authenticate_user! before filter to check if the user is authenticated. It is skipped on public pages. If a user is not authenticated it redirects to sessions#new action which redirects to the CAS authentication or to the sign-in page which is a sessions#new view.

Past the sign in page CAS authentication system does not interact at all with devise controllers. They are assigned a random 20 character password and their e-mail confirmation is skipped.

The Devise users on the other hand use new, create, and update actions of the devise controller that I have overridden in order to update e-mails and logins properly. All Devise controllers are located inside the gem but the ones that I am overriding are contained in the /controllers/users/ folder. Their views are in the subfolders of the view/users folders, and the rest of the views, controllers for which aren't overridden are in the views/devise folder.

The admin user is auto-confirmed but this can be changed in the application_setup controller, all other users that are created are sent a confirmation e-mail, and have to confirm before they can sign in. They can be given a grace period by uncommenting a line in the devise initializer and setting a time period.

If Admin edits users profiles regular users CRUD actions are called rather than the registrations controller ones.

I don't see any immediate problems other than if the LDAP server settings are not correctly entered search_ldap function will fail and die a horrible death, and bring down the rest of everything so that should probably be handled.

In general devise modules are added and removed in the users.rb model and the routes are managed with the devise_for method in the routes.rb. there I have skipped registrations module and manually defined routes for certain calls to prevent devise from overriding old users CRUD actions.

Also Devise provides its own current_user method so I deleted our old one and the helpers.

App_setup works great if you don't do silly things to it but I wouldn't consider it bomb-proof.

Although I won't have my computer with me in Peru I will have my phone and hopefully some form of WiFi so I will be able to answer questions via e-mail if you have any.

@orenyk
Copy link
Contributor

orenyk commented May 13, 2014

It looks like a lot of work was done with this a while ago; not sure how much of it is still usable. I'm wondering if it might be possible to have CAS as an authentication option along with others, that would probably be best so that it's usable outside of Yale, but there are a lot of other Yale-specific aspects of the app we might want to generalize out. That's discussion for a different issue, leaving this open for further discussion.

@dgoerger
Copy link
Contributor

https://github.com/dlindahl/omniauth-cas (?)

(solves the Rails4 upgrade problem of RubyCAS #585)

(I'll try it out on YaleSTC/pulse and report back!)

@orenyk orenyk mentioned this issue Jul 15, 2014
13 tasks
@dgoerger
Copy link
Contributor

omniauth-cas is a no-go. versions / API-compatibility appears incorrect and it fails to parse the CAS ticket as legitimate. :'(

@dgoerger
Copy link
Contributor

@mnquintana
Copy link
Contributor

This might be even better: https://github.com/biola/rack-cas

(Found via rubycas/rubycas-client#78 (comment) - recommended by soupmatt)

It runs on Rack, the middleware that runs Rails, so it isn't dependent on Rails or a Rails-dependent gem like the other solutions. This might be the one.

@orenyk
Copy link
Contributor

orenyk commented Oct 24, 2014

WOOOOOOOOOOOT! Made some updates to the #create method and updated the user form, and switching between CAS and non-CAS works beautifully :-)

@orenyk
Copy link
Contributor

orenyk commented Oct 24, 2014

Also, I merged in the recent changes to master, including integration testing, but it looks like the Devise test helpers are designed for controller tests, not feature tests. I'll look into following this article next to set up working integration tests.

EDIT basically done, I'd just like to move the call to Warden out of each specific file (see spec/features/login_spec.rb)

@orenyk
Copy link
Contributor

orenyk commented Oct 26, 2014

I spent too long trying to get Announcement integration tests working, but now moving on to configuration. I think we're going to skip the :rememberable and :timeoutable: module, but :trackable can't really hurt. I'll get that set up and then clean up the devise.rb file.

@orenyk
Copy link
Contributor

orenyk commented Oct 26, 2014

Ok, :trackable has been set up for both authentication methods and :recoverable has been activated for the non-CAS case, but I haven't tested it out at all. Here's my remaining list of things to do (that needs to be transferred to the description above):

TRANSFERRED

@orenyk orenyk closed this as completed Oct 26, 2014
@orenyk orenyk reopened this Oct 26, 2014
@orenyk
Copy link
Contributor

orenyk commented Oct 26, 2014

Ok, I tested the :recoverable was working as expected and updated the Devise views to fit in our app better. I also deleted all the other unused Devise views. Now all that's left should be testing and documentation!

@orenyk
Copy link
Contributor

orenyk commented Oct 26, 2014

Note that the currently failing tests have to do with the modifications I made to the User model when we're not using CAS. We should either set ENV['CAS_AUTH'] or update our tests. Really, we should test both cases... I'll look into change environment variables in before blocks to see how to do that.

EDIT This looks awesome! (also see this)

@orenyk
Copy link
Contributor

orenyk commented Oct 27, 2014

I've taken out the :trackable module since it was messing with out CAS new user workflow (saving the user when saving the login tracking fields)

@orenyk
Copy link
Contributor

orenyk commented Oct 27, 2014

Whew, I think this is basically done. We probably want to add tests for editing users to make sure that respect the rules for editing you own profile vs someone elses and being able to change your own password, but we can leave some of that for #416. Also, we should probably write proper tests for the password reset functionality but I don't really have time to do it and I don't think it's worth it since we're purely using Devise. I'll open a PR now, but it doesn't need to be reviewed until editing functionality has been tested.

@orenyk
Copy link
Contributor

orenyk commented Oct 27, 2014

I've started writing the profile specs. Once they're done (hopefully tomorrow), this will be ready for review!

@orenyk orenyk modified the milestones: 4.1.0, 4.0.1 Oct 27, 2014
@orenyk
Copy link
Contributor

orenyk commented Oct 29, 2014

Ok, wrote a bunch of profile specs. There are still two intermittently failing AppConfigController tests that will sometimes fail both locally and on Travis and then randomly pass. That said, I think this is ready to go :-D!

@shippy
Copy link
Contributor

shippy commented Oct 30, 2014

I just want to say that this is a triumph; I'm making a note here, "huge success". It's hard to overstate my satisfaction.

@orenyk
Copy link
Contributor

orenyk commented Oct 31, 2014

Thanks @shippy, I really appreciate it :-). Feel free to take a look at the PR if you have free time :-P.

@orenyk orenyk closed this as completed in 7374131 Nov 25, 2014
AlanLiu96 added a commit that referenced this issue Feb 12, 2015
AlanLiu96 added a commit that referenced this issue Feb 23, 2015
Changed .nil? on notes to .blank?
added markdown to the notes that were missing it
(still need to test)

Signed-off-by: Alan Liu <[email protected]>

Added an absolute link to the reservations in checkin receipt, checkout receipt, checked_in_fine email, and reservation confirmation.

Added tests to those four emails to see if they contain a link with reservations in it.

fixed travis offenses

removed extra spaces, reverted a few changes to requirements notes,

improved the test by adding in the full url. added a custom test for one case because @res is an array

travis

travis try #2
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants