fernet / fernet-rb

Delicious HMAC Digest(if) authentication and AES-128-CBC encryption
MIT License
89 stars 22 forks source link

Move call to super to the beginning of the method #22

Closed josyan closed 11 years ago

josyan commented 11 years ago

Valcro's own validate method clear all errors before executing, and so it clears too all Token's errors, resulting in a always valid token, no matters its content.

I ensure Token's validation are executed after the Valcro's validators.

hgmnz commented 11 years ago

Very intersting. Thanks for the patch! Need to reproduce the issue in a spec however. Looking into that.

josyan commented 11 years ago

Thanks. Related to specs, it look like its missing some files under spec/fernet-spec to run the specs (like invalid.json, for example.)

hgmnz commented 11 years ago

I see the problem conceptually. Can you paste some code here that reproduced the problem, eg: how did this affect you in your code?

hgmnz commented 11 years ago

it look like its missing some files under spec/fernet-spec to run the specs (like invalid.json, for example.)

Per the readme, try git submodule init. invalid.json comes from a different repo that is included here via a submodule.

josyan commented 11 years ago

I am using fernet to authenticate calls to an API between servers. In my controller, I have some code like that:

raise 'access_denied' unless Fernet.verifier('some_base_64_key', params[:token]).valid?

When calling my api with a standard post

POST http://www.host.com/api/service?token=abc

the validation always pass, and the error is not raised, when I know that abc is not a valid token.

I checked the valid? method in the Token class, which call the validate method. Some debugging code told me that errors really where added, but after the call to super in validate, I had no more errors. I checked the Valcro#validate code, where I can see that errors are cleared.

hgmnz commented 11 years ago

I see. Are you using the released fernet version, or are you tying it to this repo in your Gemfile? You may be using an older fernet. (What's Fernet::VERSION ?)

In any case, a while ago I added a nicer method in valcro for defining validation. The purpose is to decouple knowledge of having to call super at all.

I am switching to using that instead: https://github.com/hgmnz/fernet/pull/23

josyan commented 11 years ago

Fernet::VERSION => "1.6", in the Gemfile the fernet gem is tied to git://github.com/hgmnz/fernet.git, so I think its on master, and not on the v1.6 tag. It has been used in our project for some month, but only to generate tokens, not to validate them.

I am going to try the v1.6 tag, adjust my code accordingly, and so you can probably ignore my pull request.

hgmnz commented 11 years ago

Hmm, if you're tied to this repo, then that's the latest. Fernet::VERSION has not been updated in master just yet, so it's not telling us all that much. I suggest you don't try the tag just yet, but instead bind it to the master's HEAD by using the exact commit. Can you please try that? That would also include my latest patch as of a few minutes ago.

Thanks

josyan commented 11 years ago

Tried that, it seems to work fine now! Thanks!

hgmnz commented 11 years ago

woo!

Thanks for working with me on this.

A release candidate of fernet (2.0.rc1) is forthcoming soon.