Fix OAuth Scope in GitHub

* Adds a OAuth scope header for each request * Fixes UT Closes #123
silexlabs · Dec 14, 2017 · ae70153 · ae70153
1 parent ff81036
commit ae70153
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 28 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,8 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
+### Fixed
+- [GitHub] OAuth scopes are provided in request headers (https://github.com/silexlabs/unifile/issues/123)
 
 ## [2.0.1] - 2017-12-11
 ### Fixed

diff --git a/lib/unifile-github.js b/lib/unifile-github.js
@@ -1,6 +1,5 @@
 'use strict';
 
-const Path = require('path');
 const Url = require('url');
 const {Writable, Transform, PassThrough} = require('stream');
 
@@ -370,7 +369,7 @@ class GitHubConnector {
 				};
 				return this[callAPI](session, apiPath, reqData, 'POST')
 				// Renames default README to a more discreet .gitkeep
-				.then(() => this.rename(session, Path.join(path, 'master/README.md'), Path.join(path, 'master/.gitkeep')));
+				.then(() => this.rename(session, path + '/master/README.md', path + '/master/.gitkeep'));
 			case 2: // Create a branch
 				return this[createBranch](session, splitPath[0], splitPath[1]);
 			default: // Create a folder (with a .gitkeep file in it because git doesn't track empty folder)
@@ -467,7 +466,7 @@ class GitHubConnector {
 				if(res.type === 'file') {
 					return Buffer.from(res.content, res.encoding);
 				} else {
-					return Promise.reject(new UnifileError(UnifileError.EISDIR, 'This is a folder.'));
+					return Promise.reject(new UnifileError(UnifileError.EISDIR, 'Path is a directory.'));
 				}
 			});
 		}
@@ -559,9 +558,7 @@ class GitHubConnector {
 					if(branches.length > 1)
 						return this[callAPI](session, repoPath + '/git/refs/heads/' + splitPath[1], null, 'DELETE');
 					else {
-						const err = new Error('You cannot leave this folder empty.');
-						err.statusCode = 400;
-						throw err;
+						throw new UnifileError(UnifileError.INVAL, 'You cannot leave this folder empty.');
 					}
 				});
 			default: // Remove file/folder
@@ -880,7 +877,8 @@ class GitHubConnector {
 			headers: {
 				'Accept': 'application/vnd.github.v3+json',
 				'Authorization': session.token,
-				'User-Agent': 'Unifile'
+				'User-Agent': 'Unifile',
+				'X-OAuth-Scopes': 'delete_repo, repo, user'
 			}
 		};
 
@@ -894,20 +892,19 @@ class GitHubConnector {
 					if(err) {
 						return reject(err);
 					}
-					if(res.statusCode === 409 && JSON.parse(body).message.toLowerCase() === 'git repository is empty.' && retry) {
-						return this[callAPI](session, path, data, method, false, false)
-						.then(resolve)
-						.catch(reject);
-					} else if(res.statusCode >= 400) {
-						console.log('request failed', method, path, data, res.statusCode, body);
-						const code = (() => {
+					if(res.statusCode >= 400) {
+						const {code, message} = (() => {
+							const defaultMessage = JSON.parse(body).message;
 							switch (res.statusCode) {
-								case 403: return UnifileError.EACCES;
-								case 404: return UnifileError.ENOENT;
-								default: return UnifileError.EIO;
+								case 403:
+									return {code: UnifileError.EACCES, message: defaultMessage};
+								case 404:
+									return {code: UnifileError.ENOENT, message: 'Not Found'};
+								default:
+									return {code: UnifileError.EIO, message: defaultMessage};
 							}
 						})();
-						return reject(new UnifileError(code, JSON.parse(body).message));
+						return reject(new UnifileError(code, message));
 					}
 					try {
 						const result = res.statusCode !== 204 ? JSON.parse(body) : null;

diff --git a/test/unifile-github.js b/test/unifile-github.js
@@ -22,7 +22,7 @@ const githubDefaultInfos = {
 
 function isEnvValid() {
 	// For now, deactivate live tests because they're super slow and buggy
-	return false;//process.env.GITHUB_SECRET && process.env.GITHUB_TOKEN;
+	return process.env.GITHUB_SECRET && process.env.GITHUB_TOKEN;
 }
 
 function checkSession(session) {
@@ -33,7 +33,7 @@ function checkSession(session) {
 
 describe('GitHubConnector', function() {
 	this.slow(500);
-	this.timeout(20000);
+	this.timeout(30000);
 
 	const session = {};
 	const defaultConfig = {
@@ -540,7 +540,9 @@ describe('GitHubConnector', function() {
 		it('writes into a file', function() {
 			return connector.writeFile(session, 'unifile_writeFile/test/testFile', data)
 			.then(() => {
-				return connector.readFile(session, 'unifile_writeFile/test/testFile').should.become(data);
+				return connector.readFile(session, 'unifile_writeFile/test/testFile').then((content) => {
+					return expect(content.toString()).to.equal(data);
+				});
 			})
 			.then(() => {
 				return connector.unlink(session, 'unifile_writeFile/test/testFile');
@@ -595,7 +597,7 @@ describe('GitHubConnector', function() {
 			stream.on('close', () => {
 				return connector.readFile(session, 'unifile_writeStream/test/testStream')
 				.then((result) => {
-					expect(result).to.equal(data);
+					expect(result.toString()).to.equal(data);
 					done();
 				})
 				// Needed because the promise would catch the expect thrown exception
@@ -635,7 +637,7 @@ describe('GitHubConnector', function() {
 		it('rejects the promise if the path is a repo or branch', function() {
 			return Promise.all(['unifile_readFile', 'unifile_readFile/master'].map((path) => {
 				return expect(connector.readFile(session, path)).to.be
-				.rejectedWith('This folder only contain folders. Files can be found in sub-folders.');
+				.rejectedWith('This folder only contain folders.');
 			}));
 		});
 
@@ -647,7 +649,7 @@ describe('GitHubConnector', function() {
 			return connector.readFile(session, 'unifile_readFile/test/file1.txt')
 			.then((content) => {
 				expect(content.toString()).to.equal(data);
-				expect(content).to.be.an.instanceof(Buffer);
+				return expect(content).to.be.an.instanceof(Buffer);
 			});
 		});
 
@@ -782,7 +784,9 @@ describe('GitHubConnector', function() {
 				return connector.readFile(session, 'unifile_rename/test/file1.txt').should.be.rejectedWith('Not Found');
 			})
 			.then(() => {
-				return connector.readFile(session, 'unifile_rename/test/fileB.txt').should.become(data);
+				return connector.readFile(session, 'unifile_rename/test/fileB.txt').then((content) => {
+					return expect(content.toString()).to.equal(data);
+				});
 			});
 		});
 
@@ -899,7 +903,6 @@ describe('GitHubConnector', function() {
 	});
 
 	describe('batch()', function() {
-		this.timeout(30000);
 		let connector;
 		const creation = [
 			{name: 'mkdir', path: 'tmp'},
@@ -958,7 +961,10 @@ describe('GitHubConnector', function() {
 			return connector.batch(session, creation)
 			.then(() => {
 				return Promise.all([
-					expect(connector.readFile(session, 'tmp/test/b')).to.become('aaa'),
+					connector.readFile(session, 'tmp/test/b')
+					.then((content) => {
+						return expect(content.toString()).to.equal('aaa');
+					}),
 					expect(connector.readdir(session, 'tmp3')).to.be.fulfilled
 				]);
 			})
@@ -976,7 +982,10 @@ describe('GitHubConnector', function() {
 			creation.unshift({name: 'createReadStream', path: 'a/test/unknown_file'});
 			return connector.batch(session, creation)
 			.then(() => {
-				expect(connector.readFile(session, 'tmp/test/b')).to.become('aaa');
+				connector.readFile(session, 'tmp/test/b')
+				.then((content) => {
+					return expect(content.toString()).to.equal('aaa');
+				});
 				return connector.batch(session, destruction);
 			})
 			.then(() => {