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

Enterprise GitHub using Personal Tokens #251

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
14 changes: 11 additions & 3 deletions BuildaGitServer/Base/Authentication.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,17 @@ extension ProjectAuthenticator: KeychainStringSerializable {

let comps = value.componentsSeparatedByString(":")
guard comps.count >= 4 else { throw Error.withInfo("Corrupted keychain string") }
guard let service = GitService(rawValue: comps[0]) else {
throw Error.withInfo("Unsupported service: \(comps[0])")

var service: GitService
switch comps[0] {
case GitService.GitHub.hostname():
service = GitService.GitHub
case GitService.BitBucket.hostname():
service = GitService.BitBucket
default:
service = GitService.EnterpriseGitHub(host: comps[0])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm this will be tricky when someone tries to add a service not on any supported server. Could you instead try to synchronously ping the server and detect whether it really is a GitHub Enterprise server? We'll need this anyway to support more self-hosted servers like BitBucket Server - we'll need to ping the parsed host and detect which type it is. So using the default branch of the switch is here might cause issues with error reporting (we'll let users add even completely unsupported servers, which I'd like to avoid).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got a check for this now. I don't know if it's robust enough for you, but it was the best I could come up with.

}

guard let type = ProjectAuthenticator.AuthType(rawValue: comps[2]) else {
throw Error.withInfo("Unsupported auth type: \(comps[2])")
}
Expand All @@ -55,7 +63,7 @@ extension ProjectAuthenticator: KeychainStringSerializable {
public func toString() -> String {

return [
self.service.rawValue,
self.service.hostname(),
self.username,
self.type.rawValue,
self.secret
Expand Down
2 changes: 1 addition & 1 deletion BuildaGitServer/Base/BaseTypes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class SourceServerFactory {
public func createServer(service: GitService, auth: ProjectAuthenticator?) -> SourceServerType {

if let auth = auth {
precondition(service == auth.service)
precondition(service.type() == auth.service.type())
}

return GitServerFactory.server(service, auth: auth)
Expand Down
4 changes: 4 additions & 0 deletions BuildaGitServer/Base/GitServerFactory.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ class GitServerFactory {
let baseURL = "https://api.github.com"
let endpoints = GitHubEndpoints(baseURL: baseURL, auth: auth)
server = GitHubServer(endpoints: endpoints, http: http)
case .EnterpriseGitHub:
let baseURL = "https://api.\(service.hostname())"
let endpoints = GitHubEndpoints(baseURL: baseURL, auth: auth)
server = GitHubServer(endpoints: endpoints, http: http)
case .BitBucket:
let baseURL = "https://api.bitbucket.org"
let endpoints = BitBucketEndpoints(baseURL: baseURL, auth: auth)
Expand Down
24 changes: 20 additions & 4 deletions BuildaGitServer/GitServerPublic.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,56 +12,72 @@ import Keys
import ReactiveCocoa
import Result

public enum GitService: String {
case GitHub = "github"
case BitBucket = "bitbucket"
public enum GitService {
case GitHub
case EnterpriseGitHub(host: String)
case BitBucket
// case GitLab = "gitlab"


public func type() -> String {
switch self {
case .GitHub: return "github"
case .EnterpriseGitHub: return "enterprisegithub"
case .BitBucket: return "bitbucket"
}
}

public func prettyName() -> String {
switch self {
case .GitHub: return "GitHub"
case .EnterpriseGitHub: return "EnterpriseGitHub"
case .BitBucket: return "BitBucket"
}
}

public func logoName() -> String {
switch self {
case .GitHub: return "github"
case .EnterpriseGitHub: return "enterprisegithub"
case .BitBucket: return "bitbucket"
}
}

public func hostname() -> String {
switch self {
case .GitHub: return "github.com"
case .EnterpriseGitHub(let host): return host
case .BitBucket: return "bitbucket.org"
}
}

public func authorizeUrl() -> String {
switch self {
case .GitHub: return "https://github.com/login/oauth/authorize"
case .EnterpriseGitHub: return "https://\(hostname())/login/oauth/authorize"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have an assert here as well, since we don't support GitHub Enterprise OAuth yet.

case .BitBucket: return "https://bitbucket.org/site/oauth2/authorize"
}
}

public func accessTokenUrl() -> String {
switch self {
case .GitHub: return "https://github.com/login/oauth/access_token"
case .EnterpriseGitHub: assert(false)
case .BitBucket: return "https://bitbucket.org/site/oauth2/access_token"
}
}

public func serviceKey() -> String {
switch self {
case .GitHub: return BuildasaurKeys().gitHubAPIClientId()
case .EnterpriseGitHub: assert(false)
case .BitBucket: return BuildasaurKeys().bitBucketAPIClientId()
}
}

public func serviceSecret() -> String {
switch self {
case .GitHub: return BuildasaurKeys().gitHubAPIClientSecret()
case .EnterpriseGitHub: assert(false)
case .BitBucket: return BuildasaurKeys().bitBucketAPIClientSecret()
}
}
Expand Down
83 changes: 83 additions & 0 deletions BuildaGitServerTests/EnterpriseGitHubSourceTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
//
// EnterpriseGitHubSourceTests.swift
// Buildasaur
//
// Created by Rachel Caileff on 3/8/16.
// Copyright © 2016 Honza Dvorsky. All rights reserved.
//

import Cocoa
import XCTest
@testable import BuildaGitServer
import BuildaUtils

class EnterpriseGitHubSourceTests: XCTestCase {

var github: GitHubServer!

override func setUp() {
super.setUp()

self.github = GitServerFactory.server(.EnterpriseGitHub(host: "git.mycompany.com"), auth: nil) as! GitHubServer // TODO: fill in accessible enterprise github host
}

override func tearDown() {

self.github = nil

super.tearDown()
}

func tryEndpoint(method: HTTP.Method, endpoint: GitHubEndpoints.Endpoint, params: [String: String]?, completion: (body: AnyObject!, error: NSError!) -> ()) {

let expect = expectationWithDescription("Waiting for url request")

let request = try! self.github.endpoints.createRequest(method, endpoint: endpoint, params: params)

self.github.http.sendRequest(request, completion: { (response, body, error) -> () in

completion(body: body, error: error)
expect.fulfill()
})

waitForExpectationsWithTimeout(10, handler: nil)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this going to fail outside of your network?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests are commented out. The helper functions shouldn't be a problem.

}

// func testGetPullRequests() {
//
// let params = [
// "repo": "my/repo" // TODO: fill in accessible enterprise github repo
// ]
//
// self.tryEndpoint(.GET, endpoint: .PullRequests, params: params) { (body, error) -> () in
//
// XCTAssertNotNil(body, "Body must be non-nil")
// if let body = body as? NSArray {
// let prs: [GitHubPullRequest] = GitHubArray(body)
// XCTAssertGreaterThan(prs.count, 0, "We need > 0 items to test parsing")
// Log.verbose("Parsed PRs: \(prs)")
// } else {
// XCTFail("Body nil")
// }
// }
// }
//
// func testGetBranches() {
//
// let params = [
// "repo": "my/repo" // TODO: fill in accessible enterprise github repo
// ]
//
// self.tryEndpoint(.GET, endpoint: .Branches, params: params) { (body, error) -> () in
//
// XCTAssertNotNil(body, "Body must be non-nil")
// if let body = body as? NSArray {
// let branches: [GitHubBranch] = GitHubArray(body)
// XCTAssertGreaterThan(branches.count, 0, "We need > 0 items to test parsing")
// Log.verbose("Parsed branches: \(branches)")
// } else {
// XCTFail("Body nil")
// }
// }
// }
}
2 changes: 1 addition & 1 deletion BuildaKit/SyncerManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ extension SyncerManager: HeartbeatManagerDelegate {
public func typesOfRunningSyncers() -> [String : Int] {
return self.syncers.filter { $0.active }.reduce([:]) { (all, syncer) -> [String: Int] in
var stats = all
let syncerType = syncer._project.workspaceMetadata!.service.rawValue
let syncerType = syncer._project.workspaceMetadata!.service.type()
stats[syncerType] = (stats[syncerType] ?? 0) + 1
return stats
}
Expand Down
7 changes: 4 additions & 3 deletions BuildaKit/WorkspaceMetadata.swift
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ extension WorkspaceMetadata {

let scheme = NSURL(string: urlString)!.scheme
switch scheme {
case "github.com":
case GitService.GitHub.hostname():
return (CheckoutType.SSH, .GitHub)
case "bitbucket.org":
case GitService.BitBucket.hostname():
return (CheckoutType.SSH, .BitBucket)
case "https":

Expand All @@ -93,7 +93,8 @@ extension WorkspaceMetadata {
Log.error("HTTPS or SVN not yet supported, please create an issue on GitHub if you want it added (czechboy0/Buildasaur)")
return nil
default:
return nil
var urlPieces = urlString.split(":")
return (CheckoutType.SSH, .EnterpriseGitHub(host: urlPieces[0]))
}
}
}
4 changes: 4 additions & 0 deletions Buildasaur.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@
50ADEFE7D2EF4699DED04224 /* Pods_BuildaGitServer.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 36699A7FE0D3AFD88AD7B045 /* Pods_BuildaGitServer.framework */; };
56C0A05D3B3E3581DEEF4FB0 /* Pods_BuildaKitTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 0BF6053CD3D62FC0E0D73A25 /* Pods_BuildaKitTests.framework */; };
78D6132977F05F7A73A1B808 /* Pods_Buildasaur.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 93327F5F8E340696C059C413 /* Pods_Buildasaur.framework */; };
8476D4061C8F4CD000463074 /* EnterpriseGitHubSourceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8476D4051C8F4CD000463074 /* EnterpriseGitHubSourceTests.swift */; };
C36E3439B05CEFA228E45AE3 /* Pods_BuildaGitServerTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 84C28C2D0A7294FD001BDF5B /* Pods_BuildaGitServerTests.framework */; };
CF8249D720271FDB033C7CF8 /* Pods_BuildaHeartbeatKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 72A661282067D9AFD99D4095 /* Pods_BuildaHeartbeatKit.framework */; };
/* End PBXBuildFile section */
Expand Down Expand Up @@ -399,6 +400,7 @@
5F760D488BD75B28665F9576 /* Pods-BuildaKit.testing.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-BuildaKit.testing.xcconfig"; path = "Pods/Target Support Files/Pods-BuildaKit/Pods-BuildaKit.testing.xcconfig"; sourceTree = "<group>"; };
6358BD9D54AF08FBB17D6657 /* Pods-Buildasaur.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Buildasaur.debug.xcconfig"; path = "Pods/Target Support Files/Pods-Buildasaur/Pods-Buildasaur.debug.xcconfig"; sourceTree = "<group>"; };
72A661282067D9AFD99D4095 /* Pods_BuildaHeartbeatKit.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_BuildaHeartbeatKit.framework; sourceTree = BUILT_PRODUCTS_DIR; };
8476D4051C8F4CD000463074 /* EnterpriseGitHubSourceTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = EnterpriseGitHubSourceTests.swift; sourceTree = "<group>"; };
84C28C2D0A7294FD001BDF5B /* Pods_BuildaGitServerTests.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_BuildaGitServerTests.framework; sourceTree = BUILT_PRODUCTS_DIR; };
8D9CBDB5469FB644E5584126 /* Pods-Buildasaur.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Buildasaur.release.xcconfig"; path = "Pods/Target Support Files/Pods-Buildasaur/Pods-Buildasaur.release.xcconfig"; sourceTree = "<group>"; };
93327F5F8E340696C059C413 /* Pods_Buildasaur.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Buildasaur.framework; sourceTree = BUILT_PRODUCTS_DIR; };
Expand Down Expand Up @@ -729,6 +731,7 @@
3A7577A91C593A6A0023F08C /* Data */,
3A7AF60F1C591FAD000FD726 /* BitBucketServerTests.swift */,
3A58B1581A3B96C9003E0266 /* GitHubServerTests.swift */,
8476D4051C8F4CD000463074 /* EnterpriseGitHubSourceTests.swift */,
3AAF6EF51A3CE5BA00C657FB /* Supporting Files */,
);
path = BuildaGitServerTests;
Expand Down Expand Up @@ -1622,6 +1625,7 @@
buildActionMask = 2147483647;
files = (
3A7AF6101C591FAD000FD726 /* BitBucketServerTests.swift in Sources */,
8476D4061C8F4CD000463074 /* EnterpriseGitHubSourceTests.swift in Sources */,
3AAF6F0B1A3CE82B00C657FB /* GitHubServerTests.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand Down
4 changes: 2 additions & 2 deletions Buildasaur/Base.lproj/Main.storyboard
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<document type="com.apple.InterfaceBuilder3.Cocoa.Storyboard.XIB" version="3.0" toolsVersion="10089" systemVersion="15E33e" targetRuntime="MacOSX.Cocoa" propertyAccessControl="none" useAutolayout="YES">
<document type="com.apple.InterfaceBuilder3.Cocoa.Storyboard.XIB" version="3.0" toolsVersion="10109" systemVersion="15D21" targetRuntime="MacOSX.Cocoa" propertyAccessControl="none" useAutolayout="YES">
<dependencies>
<deployment identifier="macosx"/>
<plugIn identifier="com.apple.InterfaceBuilder.CocoaPlugin" version="10089"/>
<plugIn identifier="com.apple.InterfaceBuilder.CocoaPlugin" version="10109"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please revert these changes, since you didn't change anything I'd prefer to not have a diff here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

<capability name="Aspect ratio constraints" minToolsVersion="5.1"/>
<capability name="stacking Non-gravity area distributions on NSStackView" minToolsVersion="7.0" minSystemVersion="10.11"/>
</dependencies>
Expand Down
29 changes: 20 additions & 9 deletions Buildasaur/ProjectViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,13 @@ class ProjectViewController: ConfigEditViewController {
.startWithNext { [weak self] (proj, auth, forceUseToken) in
self?.updateServiceMeta(proj, auth: auth, userWantsTokenAuth: forceUseToken)
}
combineLatest(self.tokenTextField.rac_text, self.userWantsTokenAuth.producer)
.startWithNext { [weak self] token, forceToken in
combineLatest(self.tokenTextField.rac_text, self.userWantsTokenAuth.producer, meta)
.startWithNext { [weak self] token, forceToken, meta in
if forceToken {
if token.isEmpty {
self?.authenticator.value = nil
} else {
self?.authenticator.value = ProjectAuthenticator(service: .GitHub, username: "GIT", type: .PersonalToken, secret: token)
self?.authenticator.value = ProjectAuthenticator(service: meta.service, username: "GIT", type: .PersonalToken, secret: token)
}
}
}
Expand Down Expand Up @@ -166,22 +166,33 @@ class ProjectViewController: ConfigEditViewController {

let alreadyHasAuth = auth != nil

var showTokenField = false

switch service {
case .GitHub:
case GitService.GitHub:
if let auth = auth where auth.type == .PersonalToken && !auth.secret.isEmpty {
self.tokenTextField.stringValue = auth.secret
} else {
self.tokenTextField.stringValue = ""
}
self.useTokenButton.hidden = alreadyHasAuth
case .BitBucket:
self.loginButton.hidden = alreadyHasAuth
self.logoutButton.hidden = !alreadyHasAuth
showTokenField = userWantsTokenAuth && (auth?.type == .PersonalToken || auth == nil)
case GitService.EnterpriseGitHub:
if !alreadyHasAuth {
self.tokenTextField.stringValue = ""
}
self.useTokenButton.hidden = alreadyHasAuth
self.loginButton.hidden = true
self.logoutButton.hidden = true
showTokenField = true
case GitService.BitBucket:
self.useTokenButton.hidden = true
self.loginButton.hidden = alreadyHasAuth
self.logoutButton.hidden = !alreadyHasAuth
}

self.loginButton.hidden = alreadyHasAuth
self.logoutButton.hidden = !alreadyHasAuth

let showTokenField = userWantsTokenAuth && service == .GitHub && (auth?.type == .PersonalToken || auth == nil)
self.tokenStackView.hidden = !showTokenField
}

Expand Down
2 changes: 2 additions & 0 deletions Buildasaur/ServiceAuthentication.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ class ServiceAuthenticator {
switch service {
case .GitHub:
return self.getGitHubParameters()
case .EnterpriseGitHub:
assert(false)
case .BitBucket:
return self.getBitBucketParameters()
// default:
Expand Down