search

songkick / oauth2-provider

Simple OAuth 2.0 provider toolkit
MIT License
528 stars 148 forks source link

Add appraisals support and make specs green across the board #32

Closed pixeltrix closed 12 years ago

pixeltrix commented 12 years ago

If you're serious about backwards compatibility then you should be testing against those versions. :smile:

http://travis-ci.org/#!/pixeltrix/oauth2-provider/builds/1966680

jcoglan commented 12 years ago

I've merged most of this back into master. Basically I kept all the Appraisals settings, but left anything that involved a public API change -- I don't think changes to ActiveRecord should be making our API grow.

I've also not had the problem you seem to have had with binary data, i.e. the reason you made a schema change -- how do I replicate that?

Thanks for doing this -- I didn't know about Appraisals and it's great to have a cross-platform build.

pixeltrix commented 12 years ago

I've also not had the problem you seem to have had with binary data, i.e. the reason you made a schema change -- how do I replicate that?

Not sure why you don't see it but I see the output "ERROR -- : Binary data inserted for 'string' type on column 'client_secret_hash'" to STDERR for every spec because BCrypt::Password.create(secret) returns an ASCII-8BIT encoded string which the sqlite adapter forces to UTF-8 and logs the error.

jcoglan commented 12 years ago

Ah, I must have missed one version on my local box. Ruby 1.9.2, AR 3.1 does it for me. I don't think this should mean we change the schema for everyone though -- I don't think people are using sqlite in production. What if I change it to be binary just when running the tests?

jcoglan commented 12 years ago

I've fixed it by using force_encoding on the result of bcrypt, rather than changing the schema. I've also changed the schema to use real migration formats, so we can make new migrations instead of changing the original should the need arise.

pixeltrix commented 12 years ago

I did think about that but I was wondering wether it would affect comparison later - I'll defer to your judgement as you're the one who's done a crypto course.