volatiletech / authboss

The boss of http auth.
MIT License
3.81k stars 207 forks source link

OAuth1 Module #293

Closed stephenafamo closed 4 years ago

stephenafamo commented 4 years ago

I'm wondering if you'd like a PR for adding an OAuth1 module. I had to build one for my projects to add "Twitter login". If you think it'll be nice to have in this repo along with the other modules, I'll be happy to clean up the code a bit and send in a PR.

stephenafamo commented 4 years ago

Here's a link to what I have so far. https://github.com/stephenafamo/authboss-oauth1

aarondl commented 4 years ago

I'm pretty indifferent to this :) If you'd prefer to retain full control over it I could simply link to it from the readme.

stephenafamo commented 4 years ago

The main issue with leaving it in a separate module is registering the events.

in events.go we have this:


// Event type is for describing events
type Event int

// Event kinds
const (
    EventRegister Event = iota
    EventAuth
    // EventAuthHijack is used to steal the authentication process after a
    // successful auth but before any session variable has been put in.
    // Most useful for defining an additional step for authentication
    // (like 2fa). It needs to be separate to EventAuth because other modules
    // do checks that would also interrupt event handlers with an authentication
    // failure so there's an ordering problem.
    EventAuthHijack
    EventOAuth2
    EventAuthFail
    EventOAuth2Fail
    EventRecoverStart
    EventRecoverEnd
    EventGetUser
    EventGetUserSession
    EventPasswordReset
    EventLogout
)

Currently, in the oauth1 module, I have to do this:

        EventOAuth1     authboss.Event = 23
    EventOAuth1Fail authboss.Event = 24

This feels a little brittle since I am just choosing numbers and hoping it does not conflict with another module down the line.

aarondl commented 4 years ago

That does feel a little brittle. Sort of like port allocation. But just like port allocation you can use something wild that -probably- won't conflict :) Rather than use 23/24 use 2000 2001? Or something even less likely: 2341 2342. Afaik there's nothing in the codebase that attempts to iterate over these.

stephenafamo commented 4 years ago

Alright, I'll do that and send a PR to list the OAuth1 module in the Readme if that's fine with you.

aarondl commented 4 years ago

Absolutely :) Looking forward to it.