From 509c658080d20d88bda331358815f1111e11da36 Mon Sep 17 00:00:00 2001 From: Gabe Kangas Date: Wed, 28 Jul 2021 12:37:41 -0700 Subject: [PATCH] Add OPTIONS preflight support for 3rd party auth. - Explicitly add wildcard CORS header within the middleware. - Accept all OPTIONS preflight requests within the middlware. - Add success tests for the OPTIONS request. - Add failure tests for GET requests. --- router/middleware/auth.go | 9 +++ test/automated/integrations.test.js | 108 +++++++++++++++++++++------- 2 files changed, 91 insertions(+), 26 deletions(-) diff --git a/router/middleware/auth.go b/router/middleware/auth.go index 8409e19a5..6dcef6b8e 100644 --- a/router/middleware/auth.go +++ b/router/middleware/auth.go @@ -56,6 +56,12 @@ func accessDenied(w http.ResponseWriter) { // RequireExternalAPIAccessToken will validate a 3rd party access token. func RequireExternalAPIAccessToken(scope string, handler ExternalAccessTokenHandlerFunc) http.HandlerFunc { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // We should accept 3rd party preflight OPTIONS requests. + if r.Method == "OPTIONS" { + w.WriteHeader(http.StatusOK) + return + } + authHeader := strings.Split(r.Header.Get("Authorization"), "Bearer ") token := strings.Join(authHeader, "") @@ -71,6 +77,9 @@ func RequireExternalAPIAccessToken(scope string, handler ExternalAccessTokenHand return } + // All valid 3rd party requests should have a wildcard CORS header. + w.Header().Set("Access-Control-Allow-Origin", "*") + handler(*integration, w, r) if err := user.SetExternalAPIUserAccessTokenAsUsed(token); err != nil { diff --git a/test/automated/integrations.test.js b/test/automated/integrations.test.js index 4a16cfc52..901b72cae 100644 --- a/test/automated/integrations.test.js +++ b/test/automated/integrations.test.js @@ -10,7 +10,7 @@ const events = ['CHAT']; test('create webhook', async (done) => { const res = await sendIntegrationsChangePayload('webhooks/create', { url: webhook, - events: events, + events: events, }); expect(res.body.url).toBe(webhook); @@ -20,8 +20,10 @@ test('create webhook', async (done) => { }); test('check webhooks', (done) => { - request.get('/api/admin/webhooks') - .auth('admin', 'abc123').expect(200) + request + .get('/api/admin/webhooks') + .auth('admin', 'abc123') + .expect(200) .then((res) => { expect(res.body).toHaveLength(1); expect(res.body[0].url).toBe(webhook); @@ -40,8 +42,10 @@ test('delete webhook', async (done) => { }); test('check that webhook was deleted', (done) => { - request.get('/api/admin/webhooks') - .auth('admin', 'abc123').expect(200) + request + .get('/api/admin/webhooks') + .auth('admin', 'abc123') + .expect(200) .then((res) => { expect(res.body).toHaveLength(0); done(); @@ -50,10 +54,14 @@ test('check that webhook was deleted', (done) => { test('create access token', async (done) => { const name = 'Automated integration test'; - const scopes = ['CAN_SEND_SYSTEM_MESSAGES', 'CAN_SEND_MESSAGES']; + const scopes = [ + 'CAN_SEND_SYSTEM_MESSAGES', + 'CAN_SEND_MESSAGES', + 'HAS_ADMIN_ACCESS', + ]; const res = await sendIntegrationsChangePayload('accesstokens/create', { name: name, - scopes: scopes, + scopes: scopes, }); expect(res.body.accessToken).toBeTruthy(); @@ -61,38 +69,80 @@ test('create access token', async (done) => { expect(res.body.displayName).toBe(name); expect(res.body.scopes).toStrictEqual(scopes); accessToken = res.body.accessToken; + done(); }); test('check access tokens', async (done) => { - const res = await request.get('/api/admin/accesstokens') - .auth('admin', 'abc123').expect(200) - const tokenCheck = res.body.filter((token) => token.accessToken === accessToken) + const res = await request + .get('/api/admin/accesstokens') + .auth('admin', 'abc123') + .expect(200); + const tokenCheck = res.body.filter( + (token) => token.accessToken === accessToken + ); expect(tokenCheck).toHaveLength(1); done(); }); test('send a system message using access token', async (done) => { - const payload = {body: 'This is a test system message from the automated integration test'}; - const res = await request.post('/api/integrations/chat/system') + const payload = { + body: 'This is a test system message from the automated integration test', + }; + const res = await request + .post('/api/integrations/chat/system') .set('Authorization', 'Bearer ' + accessToken) - .send(payload).expect(200); + .send(payload) + .expect(200); done(); }); test('send an external integration message using access token', async (done) => { - const payload = {body: 'This is a test external message from the automated integration test'}; - const res = await request.post('/api/integrations/chat/send') + const payload = { + body: 'This is a test external message from the automated integration test', + }; + const res = await request + .post('/api/integrations/chat/send') .set('Authorization', 'Bearer ' + accessToken) - .send(payload).expect(200); + .send(payload) + .expect(200); done(); }); test('send an external integration action using access token', async (done) => { - const payload = {body: 'This is a test external action from the automated integration test'}; - const res = await request.post('/api/integrations/chat/action') + const payload = { + body: 'This is a test external action from the automated integration test', + }; + const res = await request + .post('/api/integrations/chat/action') .set('Authorization', 'Bearer ' + accessToken) - .send(payload).expect(200); + .send(payload) + .expect(200); + done(); +}); + +test('test fetch chat history using access token', async (done) => { + const res = await request + .get('/api/integrations/chat') + .set('Authorization', 'Bearer ' + accessToken) + .expect(200); + done(); +}); + + +test('test fetch chat history failure using invalid access token', async (done) => { + const res = await request + .get('/api/integrations/chat') + .set('Authorization', 'Bearer ' + 'invalidToken') + .expect(401); + done(); +}); + +test('test fetch chat history OPTIONS request', async (done) => { + const res = await request + .options('/api/integrations/chat') + .set('Authorization', 'Bearer ' + accessToken) + .expect(200); done(); }); @@ -105,18 +155,24 @@ test('delete access token', async (done) => { }); test('check token delete was successful', async (done) => { - const res = await request.get('/api/admin/accesstokens') - .auth('admin', 'abc123').expect(200) - const tokenCheck = res.body.filter((token) => token.accessToken === accessToken) + const res = await request + .get('/api/admin/accesstokens') + .auth('admin', 'abc123') + .expect(200); + const tokenCheck = res.body.filter( + (token) => token.accessToken === accessToken + ); expect(tokenCheck).toHaveLength(0); done(); }); async function sendIntegrationsChangePayload(endpoint, payload) { const url = '/api/admin/' + endpoint; - const res = await request.post(url) + const res = await request + .post(url) .auth('admin', 'abc123') - .send(payload).expect(200); + .send(payload) + .expect(200); - return res -} \ No newline at end of file + return res; +}