ComputerSurge/shields
3
0
Fork 0
Code Pull Requests 1 Activity

Check request origin before sending credentials (#4729)

Browse Source 
Co-authored-by: Caleb Cartwright <calebcartwright@users.noreply.github.com>
Co-authored-by: Paul Melnikow <github@paulmelnikow.com>
Co-authored-by: chris48s <chris48s@users.noreply.github.com>

Co-authored-by: Caleb Cartwright <calebcartwright@users.noreply.github.com>
Co-authored-by: Paul Melnikow <github@paulmelnikow.com>
Co-authored-by: chris48s <chris48s@users.noreply.github.com>
This commit is contained in:
chris48s
2020-03-04 20:42:27 +00:00
committed by GitHub
parent c37cf7e1ee
commit d8831729cb
43 changed files with 1079 additions and 277 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
services/github/auth/acceptor.js
View File

@@ -2,6 +2,9 @@
const queryString = require('query-string')
const request = require('request')
const {
userAgent,
} = require('../../../core/base-service/legacy-request-handler')
const log = require('../../../core/server/log')
const secretIsValid = require('../../../core/server/secret-is-valid')
const serverSecrets = require('../../../lib/server-secrets')
@@ -50,7 +53,11 @@ function setRoutes({ server, authHelper, onTokenAccepted }) {
server.route(/^\/github-auth$/, (data, match, end, ask) => {
ask.res.statusCode = 302 // Found.
const query = queryString.stringify({
client_id: authHelper.user,
// TODO The `_user` property bypasses security checks in AuthHelper.
// (e.g: enforceStrictSsl and shouldAuthenticateRequest).
// Do not use it elsewhere. It would be better to clean this up so
// it's not setting a bad example.
client_id: authHelper._user,
redirect_uri: `${baseUrl}/github-auth/done`,
})
ask.res.setHeader(
@@ -71,11 +78,15 @@ function setRoutes({ server, authHelper, onTokenAccepted }) {
method: 'POST',
headers: {
'Content-type': 'application/x-www-form-urlencoded;charset=UTF-8',
'User-Agent': 'Shields.io',
'User-Agent': userAgent,
},
form: queryString.stringify({
client_id: authHelper.user,
client_secret: authHelper.pass,
// TODO The `_user` and `_pass` properties bypass security checks in
// AuthHelper (e.g: enforceStrictSsl and shouldAuthenticateRequest).
// Do not use them elsewhere. It would be better to clean
// this up so it's not setting a bad example.
client_id: authHelper._user,
client_secret: authHelper._pass,
code: data.code,
}),
}

2
services/github/auth/acceptor.spec.js
View File

@@ -16,7 +16,7 @@ const fakeShieldsSecret = 'letmeinplz'
describe('Github token acceptor', function() {
const oauthHelper = GithubConstellation._createOauthHelper({
gh_client_id: fakeClientId,
private: { gh_client_id: fakeClientId },
})
before(function() {
// Make sure properties exist.

3
services/github/github-api-provider.js
View File

@@ -3,6 +3,7 @@
const Joi = require('@hapi/joi')
const log = require('../../core/server/log')
const { TokenPool } = require('../../core/token-pooling/token-pool')
const { userAgent } = require('../../core/base-service/legacy-request-handler')
const { nonNegativeInteger } = require('../validators')
const headerSchema = Joi.object({
@@ -184,7 +185,7 @@ class GithubApiProvider {
url,
baseUrl,
headers: {
'User-Agent': 'Shields.io',
'User-Agent': userAgent,
Accept: 'application/vnd.github.v3+json',
Authorization: `token ${tokenString}`,
},

7
services/github/github-constellation.js
View File

@@ -13,14 +13,15 @@ const { setRoutes: setAcceptorRoutes } = require('./auth/acceptor')
// Convenience class with all the stuff related to the Github API and its
// authorization tokens, to simplify server initialization.
class GithubConstellation {
static _createOauthHelper(privateConfig) {
static _createOauthHelper(config) {
return new AuthHelper(
{
userKey: 'gh_client_id',
passKey: 'gh_client_secret',
authorizedOrigins: ['https://api.github.com'],
isRequired: true,
},
privateConfig
config
)
}
@@ -54,7 +55,7 @@ class GithubConstellation {
onTokenInvalidated: tokenString => this.onTokenInvalidated(tokenString),
})
this.oauthHelper = this.constructor._createOauthHelper(config.private)
this.oauthHelper = this.constructor._createOauthHelper(config)
}
scheduleDebugLogging() {