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.

Sign up for GitHub

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

Allow auth identity provider selection using provider query parameter #231

Closed
wants to merge 2 commits into from
Closed

Allow auth identity provider selection using provider query parameter #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
331f437
Add `provider` query parameter for `/auth` endpoint
ntsim Sep 12, 2018
8ba2ea3
Cleanup `auth` tests removing any excessive copy/paste
ntsim Sep 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions controllers/auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module.exports = (req, res) => {

let requestAccessToken

switch (req.params.service) {
switch (req.query.provider) {
case 'gitlab':
requestAccessToken = siteConfig =>
oauth.requestGitLabAccessToken(
Expand All @@ -34,12 +34,12 @@ module.exports = (req, res) => {
return staticman.getSiteConfig()
.then(requestAccessToken)
.then((accessToken) => {
const git = gitFactory.create(req.params.service, {
const git = gitFactory.create(req.query.provider, {
oauthToken: accessToken
})

// TODO: Simplify this when v2 support is dropped.
const getUser = req.params.version === '2' && req.params.service === 'github'
const getUser = req.params.version === '2' && req.query.provider === 'github'
? git.api.users.get({}).then(({data}) => data)
: git.getCurrentUser()

Expand Down
32 changes: 16 additions & 16 deletions coverage/cobertura-coverage.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" ?>
<!DOCTYPE coverage SYSTEM "http://cobertura.sourceforge.net/xml/coverage-04.dtd">
<coverage lines-valid="786" lines-covered="686" line-rate="0.8728" branches-valid="360" branches-covered="278" branch-rate="0.7722" timestamp="1534078804178" complexity="0" version="0.1">
<coverage lines-valid="786" lines-covered="686" line-rate="0.8728" branches-valid="360" branches-covered="278" branch-rate="0.7722" timestamp="1536784015619" complexity="0" version="0.1">
<sources>
<source>/home/nick/Development/Javascript/staticman</source>
</sources>
Expand Down Expand Up @@ -669,9 +669,9 @@
<line number="10" hits="20"/>
</lines>
</method>
<method name="(anonymous_1)" hits="61" signature="()V">
<method name="(anonymous_1)" hits="64" signature="()V">
<lines>
<line number="13" hits="61"/>
<line number="13" hits="64"/>
</lines>
</method>
<method name="(anonymous_2)" hits="10" signature="()V">
Expand Down Expand Up @@ -778,12 +778,12 @@
<line number="7" hits="53" branch="false"/>
<line number="8" hits="53" branch="false"/>
<line number="10" hits="53" branch="false"/>
<line number="14" hits="61" branch="false"/>
<line number="16" hits="61" branch="false"/>
<line number="25" hits="61" branch="true" condition-coverage="100% (2/2)"/>
<line number="14" hits="64" branch="false"/>
<line number="16" hits="64" branch="false"/>
<line number="25" hits="64" branch="true" condition-coverage="100% (2/2)"/>
<line number="26" hits="4" branch="false"/>
<line number="30" hits="57" branch="true" condition-coverage="100% (2/2)"/>
<line number="31" hits="56" branch="false"/>
<line number="30" hits="60" branch="true" condition-coverage="100% (2/2)"/>
<line number="31" hits="59" branch="false"/>
<line number="36" hits="1" branch="false"/>
<line number="41" hits="10" branch="false"/>
<line number="48" hits="1" branch="false"/>
Expand Down Expand Up @@ -815,9 +815,9 @@
</class>
<class name="GitLab.js" filename="lib/GitLab.js" line-rate="0.8649" branch-rate="0.6923">
<methods>
<method name="(anonymous_0)" hits="24" signature="()V">
<method name="(anonymous_0)" hits="21" signature="()V">
<lines>
<line number="11" hits="24"/>
<line number="11" hits="21"/>
</lines>
</method>
<method name="(anonymous_1)" hits="15" signature="()V">
Expand Down Expand Up @@ -928,11 +928,11 @@
<line number="6" hits="47" branch="false"/>
<line number="7" hits="47" branch="false"/>
<line number="8" hits="47" branch="false"/>
<line number="12" hits="24" branch="false"/>
<line number="14" hits="24" branch="true" condition-coverage="100% (2/2)"/>
<line number="12" hits="21" branch="false"/>
<line number="14" hits="21" branch="true" condition-coverage="100% (2/2)"/>
<line number="15" hits="3" branch="false"/>
<line number="19" hits="21" branch="true" condition-coverage="100% (2/2)"/>
<line number="20" hits="20" branch="false"/>
<line number="19" hits="18" branch="true" condition-coverage="100% (2/2)"/>
<line number="20" hits="17" branch="false"/>
<line number="25" hits="1" branch="false"/>
<line number="30" hits="15" branch="true" condition-coverage="75% (3/4)"/>
<line number="31" hits="15" branch="false"/>
Expand Down Expand Up @@ -1093,8 +1093,8 @@
<line number="4" hits="77" branch="false"/>
<line number="6" hits="77" branch="false"/>
<line number="7" hits="99" branch="true" condition-coverage="100% (2/2)"/>
<line number="9" hits="8" branch="false"/>
<line number="11" hits="91" branch="false"/>
<line number="9" hits="5" branch="false"/>
<line number="11" hits="94" branch="false"/>
</lines>
</class>
<class name="Logger.js" filename="lib/Logger.js" line-rate="0" branch-rate="0">
Expand Down
127 changes: 42 additions & 85 deletions test/unit/controllers/auth.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,10 @@ beforeEach(() => {

describe('Auth controller', () => {
describe('GitHub', () => {
test('authenticates to GitHub with the given code and returns the authenticated user', () => {
const mockAccessToken = 'qwertyuiop'
const mockCode = '1q2w3e4r'
const mockUser = {
login: 'johndoe',
name: 'John Doe',
email: 'johndoe@test.com'
}

const siteConfig = helpers.getConfig()
const siteConfig = helpers.getConfig()
const mockCode = '1q2w3e4r'

const oauthRequest =
nock(/github\.com/)
.post('/login/oauth/access_token')
.query({
Expand All @@ -37,6 +30,16 @@ describe('Auth controller', () => {
code: mockCode,
redirect_uri: siteConfig.get('githubAuth.redirectUri')
})

test('authenticates to GitHub with the given code and returns the authenticated user', () => {
const mockAccessToken = 'qwertyuiop'
const mockUser = {
login: 'johndoe',
name: 'John Doe',
email: 'johndoe@test.com'
}

oauthRequest
.reply(200, {
access_token: mockAccessToken
})
Expand All @@ -50,7 +53,8 @@ describe('Auth controller', () => {

const reqWithQuery = Object.assign({}, req, {
query: {
code: mockCode
code: mockCode,
provider: 'github'
}
})

Expand All @@ -64,21 +68,11 @@ describe('Auth controller', () => {

test('authenticates to GitHub with the given code and returns the original GitHub user when using v2 API', () => {
const mockAccessToken = 'qwertyuiop'
const mockCode = '1q2w3e4r'
const mockUser = {
login: 'johndoe'
}

const siteConfig = helpers.getConfig()

nock(/github\.com/)
.post('/login/oauth/access_token')
.query({
client_id: siteConfig.get('githubAuth.clientId'),
client_secret: siteConfig.get('githubAuth.clientSecret'),
code: mockCode,
redirect_uri: siteConfig.get('githubAuth.redirectUri')
})
oauthRequest
.reply(200, {
access_token: mockAccessToken
})
Expand All @@ -92,11 +86,11 @@ describe('Auth controller', () => {

const reqWithQuery = Object.assign({}, req, {
params: {
service: 'github',
version: '2'
},
query: {
code: mockCode
code: mockCode,
provider: 'github'
}
})

Expand All @@ -108,25 +102,12 @@ describe('Auth controller', () => {
})

test('returns a 401 response when unable to get an access token from GitHub', () => {
const mockCode = '1q2w3e4r'
const siteConfig = helpers.getConfig()

nock(/github\.com/)
.post('/login/oauth/access_token')
.query({
client_id: siteConfig.get('githubAuth.clientId'),
client_secret: siteConfig.get('githubAuth.clientSecret'),
code: mockCode,
redirect_uri: siteConfig.get('githubAuth.redirectUri')
})
oauthRequest
.reply(401, {
error: 'invalid_code'
})

const reqWithQuery = Object.assign({}, req, {
params: {
service: 'github'
},
query: {
code: mockCode
}
Expand Down Expand Up @@ -167,7 +148,8 @@ describe('Auth controller', () => {

const reqWithQuery = Object.assign({}, req, {
query: {
code: mockCode
code: mockCode,
provider: 'github'
}
})

Expand All @@ -180,17 +162,10 @@ describe('Auth controller', () => {
})

describe('GitLab', () => {
test('authenticates to GitLab with the given code and returns the authenticated user', () => {
const mockAccessToken = 'qwertyuiop'
const mockCode = '1q2w3e4r'
const mockUser = {
username: 'johndoe',
name: 'John Doe',
email: 'johndoe@test.com'
}

const siteConfig = helpers.getConfig()
const siteConfig = helpers.getConfig()
const mockCode = '1q2w3e4r'

const oauthRequest =
nock(/gitlab\.com/)
.post('/oauth/token')
.query({
Expand All @@ -200,6 +175,16 @@ describe('Auth controller', () => {
grant_type: 'authorization_code',
redirect_uri: siteConfig.get('gitlabAuth.redirectUri')
})

test('authenticates to GitLab with the given code and returns the authenticated user', () => {
const mockAccessToken = 'qwertyuiop'
const mockUser = {
username: 'johndoe',
name: 'John Doe',
email: 'johndoe@test.com'
}

oauthRequest
.reply(200, {
access_token: mockAccessToken
})
Expand All @@ -213,11 +198,9 @@ describe('Auth controller', () => {
.reply(200, mockUser)

const reqWithQuery = Object.assign({}, req, {
params: {
service: 'gitlab'
},
query: {
code: mockCode
code: mockCode,
provider: 'gitlab'
}
})

Expand All @@ -230,28 +213,15 @@ describe('Auth controller', () => {
})

test('returns a 401 response when unable to get an access token from GitLab', () => {
const mockCode = '1q2w3e4r'
const siteConfig = helpers.getConfig()

nock(/gitlab\.com/)
.post('/oauth/token')
.query({
client_id: siteConfig.get('gitlabAuth.clientId'),
client_secret: siteConfig.get('gitlabAuth.clientSecret'),
code: mockCode,
grant_type: 'authorization_code',
redirect_uri: siteConfig.get('gitlabAuth.redirectUri')
})
oauthRequest
.reply(401, {
error: 'invalid_code'
})

const reqWithQuery = Object.assign({}, req, {
params: {
service: 'gitlab'
},
query: {
code: mockCode
code: mockCode,
provider: 'gitlab'
}
})

Expand All @@ -264,19 +234,8 @@ describe('Auth controller', () => {

test('returns a 401 response when an incorrect access token is used for the GitLab API', () => {
const mockAccessToken = 'qwertyuiop'
const mockCode = '1q2w3e4r'

const siteConfig = helpers.getConfig()

nock(/gitlab\.com/)
.post('/oauth/token')
.query({
client_id: siteConfig.get('gitlabAuth.clientId'),
client_secret: siteConfig.get('gitlabAuth.clientSecret'),
code: mockCode,
grant_type: 'authorization_code',
redirect_uri: siteConfig.get('gitlabAuth.redirectUri')
})
oauthRequest
.reply(200, {
access_token: mockAccessToken
})
Expand All @@ -292,11 +251,9 @@ describe('Auth controller', () => {
})

const reqWithQuery = Object.assign({}, req, {
params: {
service: 'gitlab'
},
query: {
code: mockCode
code: mockCode,
provider: 'gitlab'
}
})

Expand Down