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
@auth directive and context API #169
Conversation
… are mocked (such as in graphql/__tests__/insights)
Whenever there will be a dependency to redisClient, call jest.mock().
Pull Request Test Coverage Report for Build 757
💛 - Coveralls |
Accidentally closed by removing graphql branch. |
let userContext = null; | ||
if (userId && nonce) { | ||
const context = await redis.get(userId); | ||
if (context && context.nonce === nonce) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do we generate nonce
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not included in this PR, plan to revisit this logic when implementing LIFF.
Actually I think I will use JWT (introduced in #172 ) to replace nonce, since
- JWT includes a signature so it cannot be forged
- We can mix data into JWT for client-side use, such as expire timestamp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I found that the recommended way for LINE to make sure a request come from LIFF is via token validation:
- LIFF gets access token via
getAccessToken
API - When LIFF invokes server API, attach the access token
- API server verifies if the token is valid with LINE via social API
/oauth2/v2.1/verify?access_token=
Ref: https://www.facebook.com/groups/linebot/permalink/2489421594721745/
In this way, server do not need to send any form of nonce
/ JWT to LIFF using URL params. Instead, it is LINE server that generates a token (to LIFF) and is asked to verify (from API server).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note:
In the end I decided to use JWT, implemented in this PR: https://github.com/cofacts/rumors-line-bot/pull/178/files
The reason why I use JWT is that I need to preserve state in the JWT, it's not something that LINE server knows, thus access tokens from LINE does not meet mu need.
type Query { | ||
insights: MessagingAPIInsight | ||
|
||
# Current user's chatbot context | ||
context: UserContext @auth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
context is a little bit confusing with apollo context
@@ -123,3 +128,16 @@ enum MessagingAPIInsightStatus { | |||
UNREADY | |||
OUT_OF_SERVICE | |||
} | |||
|
|||
type UserContext { | |||
state: String |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to specify non-nullable
state: String | |
state: String! | |
issuedAt: Float! | |
data: StateData! |
Since there are 9 PRs after this PR, I collect the enhancements in #183 and will address in future PRs. |
This PR implements a GraphQL API,
Query.context
, that will replace the current/context/:userId
API endpoint.As designed in design doc, we authenticate the user by
Authorization
header. WhenuserId
andnonce
match the one in Redis, we populate GraphQL context with{userContext: <the one in redis>}
.The
Query.context
API is guarded using@auth
directive, which checksuserContext
field in GraphQL context. When accessing fields with@auth
directive, it throwsAuthenticationError
ifuserContext
does not exist.References