search

rbCAS / CASino

CASino is a Ruby-based Single Sign-On solution supporting the CAS standard
MIT License
331 stars 189 forks source link

Update engine to Rails 4 #37

Closed dbackeus closed 10 years ago

dbackeus commented 10 years ago

All specs passing on rails 4.0.3.

Also removes .ruby-gemset and .ruby-version. As long as there is no clear documented target ruby version that casinoapp must run on it makes sense to let the developer run whatever version he wants and catch any potential incompatibilities through failing specs.

fellipebrito commented 10 years ago

Is there any hope it will be merged soon?

pencil commented 10 years ago

Didn't have time to look into this… But isn't just removing mass_assignment_sanitizer the wrong approach? Shouldn't it be replaced with strong_parameters‎?

jetheredge commented 10 years ago

I looked through the source, and from the controllers you're always passing the raw parameters hash to a processor, which in turn picks off the parameters from the hash that it wants and then performs the operation. It doesn't seem like you're ever just passing the params hash directly into a model constructor, which is where strong parameters would enforce anything.

I would think that due to the pattern you're following, you would not want to filter the params with strong parameters since you would probably want to get an exception if someone passes the raw params hash to an a model constructor.

In theory you could use strong parameters to enforce that all of your fields you expect to be posted are in the post, but I would imagine that this would just duplicate effort since you are picking off the individual params out of the hash in the processors.

Thoughts?

pencil commented 10 years ago

You are right, @jetheredge. Will look into this soon™.

dbackeus commented 10 years ago

Yes that's also the conclusion I reached. If there had been any insecure passing of the hash there would have been errors raised in the app as well which would have been noticed in the specs as well.

fellipebrito commented 10 years ago

Do you guys need any help in order to update it to Rails 4? I'm following this discussion, but it looks more as a taste discussion than a problem that can be solved with code.

@pencil is your plan to update that, or should we fork it and try to move on by ourselves?

Thanks

pencil commented 10 years ago

It will be merged, but I'd like to do some more testing in our staging environment beforehand. Don't want to break things at this point. ETA is next week.

fellipebrito commented 10 years ago

Thanks man, I appreciate that.

Let me know if you need any help.

jcaudle commented 10 years ago

@pencil Do you expect you'll release the merged version of this as a beta or will it just be a major version update?

pencil commented 10 years ago

@jcaudle Not sure if we will release a beta version, but the final version will definitely be a major version update.

pencil commented 10 years ago

Didn't forget you guys… Will have time to look into this by the end of the week.

fellipebrito commented 10 years ago

Thanks!

On Mon, Apr 14, 2014 at 11:49 AM, Nils Caspar notifications@github.comwrote:

Didn't forget you guys... Will have time to look into this by the end of the week.

Reply to this email directly or view it on GitHubhttps://github.com/rbCAS/CASino/pull/37#issuecomment-40403467 .

pencil commented 10 years ago

I just released CASino 3.0.0.pre.1. I updated the Rails dependency to ~> 4.1 as there seem to be only minor changes compared to 4.x. Please help us test and report back with your results :smile:

fernandomantoan commented 10 years ago

Nice.

pencil commented 10 years ago

Finally released CASino 3.0. Good things come to those who wait. :beers: