Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve API sanity #20

Merged
merged 1 commit into from Jan 18, 2016
Merged

Improve API sanity #20

merged 1 commit into from Jan 18, 2016

Conversation

aeneasr
Copy link
Member

@aeneasr aeneasr commented Jan 14, 2016

This PR is aimed at resolving API inconsitencies and giving more power to signing methods

@aeneasr aeneasr added feat New feature or request. rfc A request for comments to discuss and share ideas. labels Jan 14, 2016
@aeneasr aeneasr self-assigned this Jan 14, 2016
@@ -0,0 +1,22 @@
package core
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the core package (basically all oauth2 things) now defines what the different strategies must implement. this is an upside because if we have a "supersecure" package, it could use it's own strategy signatures

@aeneasr aeneasr added this to the v0-fixes milestone Jan 14, 2016
@leetal
Copy link
Contributor

leetal commented Jan 14, 2016

This all seems good! But i think we'll need to expose (or at least discuss alternate solutions to) at least one callback the developers can "subscribe" to by passing in a function that is called right before the ACCESS token is generated. A little like Osin does, but not the whole genration, just a little part of it. Since there is no way for a developer right now to actually modify the claims (for example) that is to be returned to the callee. What i mean is that for example one resource owner grants acces to his data, the developer might just, right before that access token is returned, fetch the resource owner data from DB and inject parts of that data into the JWT claims. I really don't want to hardcode any behavior except lease times and expire times in the lib. What would be counterintuitive with regards of the actual power and core strength of Fosite. I think i'll add such callback to the JWT mechanism by adding it to the JWTEnigma struct since probably no other auth method (except open id connect) can return data in the token.

@aeneasr
Copy link
Member Author

aeneasr commented Jan 14, 2016

I think this is already solved because we pass the user session object which is of type interface!

func (h HMACSHAStrategy) GenerateAccessToken(ctx context.Context, r *http.Request, ar fosite.AccessRequester, session interface{})

If the developer wants to use JWT, he needs to choose a session that implements for example some interface. In the JWT strategy you could do:

func (h jwt.JWTStrategy) GenerateAccessToken(ctx context.Context, r *http.Request, ar fosite.AccessRequester, session interface{}) {
    jwtSession, ok := session.(*jwt.Session)
    if ; !ok {
       return err
    }
    GenerateJWTToken(jwtSession.GetClaims())
}

The developer then might do:

// in token endpoint
// ... NewAuthorizeRequest
session := jwt.NewJWTSession()
session.SetSub("peter")
// ... NewAuthorizeResponse(..., session)
// ... Writeresponse

Would that not be enough?

@leetal
Copy link
Contributor

leetal commented Jan 14, 2016

Yes, that will do for sure! :) Totally didn't think of that use for the
session, but as you said before, stateless behavior without hussle is
important!

@aeneasr
Copy link
Member Author

aeneasr commented Jan 14, 2016

ok cool, i will implement this then :) thanks for your continued feedback, i honestly appreciate it!

@aeneasr
Copy link
Member Author

aeneasr commented Jan 15, 2016

The method signatures are in gernal too long and complex. I will replace them with context variables instead

@aeneasr
Copy link
Member Author

aeneasr commented Jan 15, 2016

gob interface magic: http://play.golang.org/p/iq7iZfhiHm

@aeneasr aeneasr changed the title Refactor enigma Improve API sanity Jan 15, 2016
aeneasr pushed a commit that referenced this pull request Jan 18, 2016
@aeneasr aeneasr merged commit a0ad1f0 into master Jan 18, 2016
@aeneasr aeneasr deleted the enigma-refactor branch March 21, 2016 14:39
budougumi0617 added a commit to budougumi0617/fosite that referenced this pull request May 10, 2019
juguhan pushed a commit to juguhan/fosite that referenced this pull request May 10, 2024
dep: upgrade protobuf and go-jose versions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request. rfc A request for comments to discuss and share ideas.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants