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

@auth directive and context API #169

Merged
merged 7 commits into from May 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -40,6 +40,7 @@
"googleapis": "^36.0.0",
"grapheme-splitter": "^1.0.4",
"graphql": "^15.0.0",
"graphql-tools": "^4.0.7",
"koa": "^2.5.0",
"koa-router": "^7.4.0",
"koa-static": "^5.0.0",
Expand Down
60 changes: 60 additions & 0 deletions src/graphql/__tests__/context.js
@@ -0,0 +1,60 @@
import { gql } from '../testUtils';

it('context rejects anonymous users', async () => {
const result = await gql`
{
context {
state
}
}
`();
expect(result).toMatchInlineSnapshot(`
Object {
"data": Object {
"context": null,
},
"errors": Array [
[GraphQLError: Invalid authentication header],
],
}
`);
});

it('Returns user context', async () => {
const result = await gql`
{
context {
state
issuedAt
data {
searchedText
}
}
}
`(
{},
{
userId: 'U12345678',
userContext: {
state: 'CHOOSING_ARTICLE',
issuedAt: 1586013070089,
data: {
searchedText: 'Foo',
},
},
}
);
expect(result).toMatchInlineSnapshot(`
Object {
"data": Object {
"context": Object {
"data": Object {
"searchedText": "Foo",
},
"issuedAt": 1586013070089,
"state": "CHOOSING_ARTICLE",
},
},
}
`);
});
75 changes: 75 additions & 0 deletions src/graphql/__tests__/index.js
@@ -0,0 +1,75 @@
jest.mock('src/lib/redisClient');

import { getContext } from '../';
import redis from 'src/lib/redisClient';

beforeEach(() => {
redis.get.mockClear();
});

describe('getContext', () => {
it('generates null context for anonymous requests', async () => {
const context = await getContext({ ctx: { req: { headers: {} } } });

expect(context).toMatchInlineSnapshot(`
Object {
"userContext": null,
"userId": "",
}
`);
});

it('generates null context for wrong credentials', async () => {
redis.get.mockImplementationOnce(() => ({
nonce: 'correctpass',
}));

const context = await getContext({
ctx: {
req: {
headers: {
authorization: `basic ${Buffer.from('user1:wrongpass').toString(
'base64'
)}`,
},
},
},
});

expect(context).toMatchInlineSnapshot(`
Object {
"userContext": null,
"userId": "user1",
}
`);
});

it('reads userContext for logged-in requests', async () => {
redis.get.mockImplementationOnce(() => ({
nonce: 'correctpass',
foo: 'bar',
}));

const context = await getContext({
ctx: {
req: {
headers: {
authorization: `basic ${Buffer.from('user1:correctpass').toString(
'base64'
)}`,
},
},
},
});

expect(context).toMatchInlineSnapshot(`
Object {
"userContext": Object {
"foo": "bar",
"nonce": "correctpass",
},
"userId": "user1",
}
`);
});
});
1 change: 0 additions & 1 deletion src/graphql/__tests__/insights.js
@@ -1,4 +1,3 @@
jest.mock('@line/bot-sdk');
import {
mockedGetNumberOfMessageDeliveries,
mockedGetNumberOfFollowers,
Expand Down
23 changes: 23 additions & 0 deletions src/graphql/directives/auth.js
@@ -0,0 +1,23 @@
import { AuthenticationError } from 'apollo-server-koa';
import { SchemaDirectiveVisitor } from 'graphql-tools';

/**
* When field with @auth is accessed, make sure the user is logged in (i.e. `userContext` in context).
*/
class AuthDirective extends SchemaDirectiveVisitor {
visitFieldDefinition(field) {
const { resolve } = field;

field.resolve = (...args) => {
const [, , context] = args;

if (!context.userId || !context.userContext) {
throw new AuthenticationError('Invalid authentication header');
}

return resolve(...args);
};
}
}

export default AuthDirective;
40 changes: 39 additions & 1 deletion src/graphql/index.js
@@ -1,6 +1,7 @@
import fs from 'fs';
import path from 'path';
import { ApolloServer, makeExecutableSchema } from 'apollo-server-koa';
import redis from 'src/lib/redisClient';

export const schema = makeExecutableSchema({
typeDefs: fs.readFileSync(path.join(__dirname, `./typeDefs.graphql`), {
Expand All @@ -14,8 +15,45 @@ export const schema = makeExecutableSchema({
] = require(`./resolvers/${fileName}`).default;
return resolvers;
}, {}),
schemaDirectives: fs
.readdirSync(path.join(__dirname, 'directives'))
.reduce((directives, fileName) => {
directives[
fileName.replace(/\.js$/, '')
] = require(`./directives/${fileName}`).default;
return directives;
}, {}),
});

const server = new ApolloServer({ schema });
/**
* @param {{ctx: Koa.Context}}
* @returns {object}
*/
export async function getContext({ ctx: { req } }) {
const [userId, nonce] = Buffer.from(
(req.headers.authorization || '').replace(/^basic /, ''),
'base64'
)
.toString()
.split(':');

let userContext = null;
if (userId && nonce) {
const context = await redis.get(userId);
if (context && context.nonce === nonce) {
Copy link
Member

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?

Copy link
Member Author

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

  1. JWT includes a signature so it cannot be forged
  2. We can mix data into JWT for client-side use, such as expire timestamp

Copy link
Member Author

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:

  1. LIFF gets access token via getAccessToken API
  2. When LIFF invokes server API, attach the access token
  3. 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).

Copy link
Member Author

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.

userContext = context;
}
}

return {
userId,
userContext,
};
}

const server = new ApolloServer({
schema,
context: getContext,
});

export default server.getMiddleware();
4 changes: 4 additions & 0 deletions src/graphql/resolvers/Query.js
Expand Up @@ -3,4 +3,8 @@ export default {
// Resolvers in next level
return {};
},

context(root, args, context) {
return context.userContext;
},
};
3 changes: 3 additions & 0 deletions src/graphql/testUtils.js
@@ -1,3 +1,6 @@
// ./index.js contains imports to redisClient, which should be mocked in unit tests.
jest.mock('src/lib/redisClient');

import { graphql } from 'graphql';
import { schema } from './';

Expand Down
18 changes: 18 additions & 0 deletions src/graphql/typeDefs.graphql
@@ -1,5 +1,10 @@
directive @auth on FIELD_DEFINITION

type Query {
insights: MessagingAPIInsight

# Current user's chatbot context
context: UserContext @auth
Copy link
Member

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

}

type MessagingAPIInsight {
Expand Down Expand Up @@ -123,3 +128,16 @@ enum MessagingAPIInsightStatus {
UNREADY
OUT_OF_SERVICE
}

type UserContext {
state: String
Copy link
Member

@LucienLee LucienLee Apr 29, 2020

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

Suggested change
state: String
state: String!
issuedAt: Float!
data: StateData!

issuedAt: Float
data: StateData
}

type StateData {
searchedText: String
selectedArticleId: ID
selectedArticleText: String
selectedReplyId: ID
}
3 changes: 3 additions & 0 deletions src/lib/__mocks__/redisClient.js
@@ -0,0 +1,3 @@
export default {
get: jest.fn(),
};