Address comments on code review
This commit is contained in:
parent
7c45aca405
commit
8d3ce44682
12 changed files with 150 additions and 33 deletions
|
@ -8,7 +8,7 @@ from peewee import *
|
||||||
from data.read_slave import ReadSlaveModel
|
from data.read_slave import ReadSlaveModel
|
||||||
from sqlalchemy.engine.url import make_url
|
from sqlalchemy.engine.url import make_url
|
||||||
from urlparse import urlparse
|
from urlparse import urlparse
|
||||||
|
from util.names import urn_generator
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
@ -116,7 +116,7 @@ class TeamMemberInvite(BaseModel):
|
||||||
email = CharField(null=True)
|
email = CharField(null=True)
|
||||||
team = ForeignKeyField(Team, index=True)
|
team = ForeignKeyField(Team, index=True)
|
||||||
inviter = ForeignKeyField(User, related_name='inviter')
|
inviter = ForeignKeyField(User, related_name='inviter')
|
||||||
invite_token = CharField(default=uuid_generator)
|
invite_token = CharField(default=urn_generator(['teaminvite']))
|
||||||
|
|
||||||
|
|
||||||
class LoginService(BaseModel):
|
class LoginService(BaseModel):
|
||||||
|
|
|
@ -10,6 +10,24 @@ from data import model
|
||||||
from util.useremails import send_org_invite_email
|
from util.useremails import send_org_invite_email
|
||||||
from util.gravatar import compute_hash
|
from util.gravatar import compute_hash
|
||||||
|
|
||||||
|
def try_accept_invite(code, user):
|
||||||
|
try:
|
||||||
|
(team, inviter) = model.confirm_team_invite(code, user)
|
||||||
|
except model.DataModelException:
|
||||||
|
return None
|
||||||
|
|
||||||
|
model.delete_matching_notifications(user, 'org_team_invite', code=code)
|
||||||
|
|
||||||
|
orgname = team.organization.username
|
||||||
|
log_action('org_team_member_invite_accepted', orgname, {
|
||||||
|
'member': user.username,
|
||||||
|
'team': team.name,
|
||||||
|
'inviter': inviter.username
|
||||||
|
})
|
||||||
|
|
||||||
|
return team
|
||||||
|
|
||||||
|
|
||||||
def handle_addinvite_team(inviter, team, user=None, email=None):
|
def handle_addinvite_team(inviter, team, user=None, email=None):
|
||||||
invite = model.add_or_invite_to_team(inviter, team, user, email)
|
invite = model.add_or_invite_to_team(inviter, team, user, email)
|
||||||
if not invite:
|
if not invite:
|
||||||
|
@ -323,20 +341,11 @@ class TeamMemberInvite(ApiResource):
|
||||||
def put(self, code):
|
def put(self, code):
|
||||||
""" Accepts an invite to join a team in an organization. """
|
""" Accepts an invite to join a team in an organization. """
|
||||||
# Accept the invite for the current user.
|
# Accept the invite for the current user.
|
||||||
try:
|
team = try_accept_invite(code, get_authenticated_user())
|
||||||
(team, inviter) = model.confirm_team_invite(code, get_authenticated_user())
|
if not team:
|
||||||
except model.DataModelException:
|
|
||||||
raise NotFound()
|
raise NotFound()
|
||||||
|
|
||||||
model.delete_matching_notifications(get_authenticated_user(), 'org_team_invite', code=code)
|
|
||||||
|
|
||||||
orgname = team.organization.username
|
orgname = team.organization.username
|
||||||
log_action('org_team_member_invite_accepted', orgname, {
|
|
||||||
'member': get_authenticated_user().username,
|
|
||||||
'team': team.name,
|
|
||||||
'inviter': inviter.username
|
|
||||||
})
|
|
||||||
|
|
||||||
return {
|
return {
|
||||||
'org': orgname,
|
'org': orgname,
|
||||||
'team': team.name
|
'team': team.name
|
||||||
|
|
|
@ -12,6 +12,8 @@ from endpoints.api import (ApiResource, nickname, resource, validate_json_reques
|
||||||
license_error)
|
license_error)
|
||||||
from endpoints.api.subscribe import subscribe
|
from endpoints.api.subscribe import subscribe
|
||||||
from endpoints.common import common_login
|
from endpoints.common import common_login
|
||||||
|
from endpoints.api.team import try_accept_invite
|
||||||
|
|
||||||
from data import model
|
from data import model
|
||||||
from data.billing import get_plan
|
from data.billing import get_plan
|
||||||
from auth.permissions import (AdministerOrganizationPermission, CreateRepositoryPermission,
|
from auth.permissions import (AdministerOrganizationPermission, CreateRepositoryPermission,
|
||||||
|
@ -20,6 +22,7 @@ from auth.auth_context import get_authenticated_user
|
||||||
from auth import scopes
|
from auth import scopes
|
||||||
from util.gravatar import compute_hash
|
from util.gravatar import compute_hash
|
||||||
from util.useremails import (send_confirmation_email, send_recovery_email, send_change_email)
|
from util.useremails import (send_confirmation_email, send_recovery_email, send_change_email)
|
||||||
|
from util.names import parse_single_urn
|
||||||
|
|
||||||
import features
|
import features
|
||||||
|
|
||||||
|
@ -179,11 +182,15 @@ class User(ApiResource):
|
||||||
return user_view(user)
|
return user_view(user)
|
||||||
|
|
||||||
@nickname('createNewUser')
|
@nickname('createNewUser')
|
||||||
|
@parse_args
|
||||||
|
@query_param('inviteCode', 'Invitation code given for creating the user.', type=str,
|
||||||
|
default='')
|
||||||
@internal_only
|
@internal_only
|
||||||
@validate_json_request('NewUser')
|
@validate_json_request('NewUser')
|
||||||
def post(self):
|
def post(self, args):
|
||||||
""" Create a new user. """
|
""" Create a new user. """
|
||||||
user_data = request.get_json()
|
user_data = request.get_json()
|
||||||
|
invite_code = args['inviteCode']
|
||||||
|
|
||||||
existing_user = model.get_user(user_data['username'])
|
existing_user = model.get_user(user_data['username'])
|
||||||
if existing_user:
|
if existing_user:
|
||||||
|
@ -194,6 +201,14 @@ class User(ApiResource):
|
||||||
user_data['email'])
|
user_data['email'])
|
||||||
code = model.create_confirm_email_code(new_user)
|
code = model.create_confirm_email_code(new_user)
|
||||||
send_confirmation_email(new_user.username, new_user.email, code.code)
|
send_confirmation_email(new_user.username, new_user.email, code.code)
|
||||||
|
|
||||||
|
# Handle any invite codes.
|
||||||
|
parsed_invite = parse_single_urn(invite_code)
|
||||||
|
if parsed_invite is not None:
|
||||||
|
if parsed_invite[0] == 'teaminvite':
|
||||||
|
# Add the user to the team.
|
||||||
|
try_accept_invite(invite_code, new_user)
|
||||||
|
|
||||||
return 'Created', 201
|
return 'Created', 201
|
||||||
except model.TooManyUsersException as ex:
|
except model.TooManyUsersException as ex:
|
||||||
raise license_error(exception=ex)
|
raise license_error(exception=ex)
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
<div class="signup-form-element">
|
<div class="signup-form-element">
|
||||||
<form class="form-signup" name="signupForm" ng-submit="register()" ngshow="!awaitingConfirmation && !registering">
|
<form class="form-signup" name="signupForm" ng-submit="register()" ng-show="!awaitingConfirmation && !registering">
|
||||||
<input type="text" class="form-control" placeholder="Create a username" name="username" ng-model="newUser.username" autofocus required ng-pattern="/^[a-z0-9_]{4,30}$/">
|
<input type="text" class="form-control" placeholder="Create a username" name="username" ng-model="newUser.username" autofocus required ng-pattern="/^[a-z0-9_]{4,30}$/">
|
||||||
<input type="email" class="form-control" placeholder="Email address" ng-model="newUser.email" required>
|
<input type="email" class="form-control" placeholder="Email address" ng-model="newUser.email" required>
|
||||||
<input type="password" class="form-control" placeholder="Create a password" ng-model="newUser.password" required
|
<input type="password" class="form-control" placeholder="Create a password" ng-model="newUser.password" required
|
||||||
|
|
|
@ -1,4 +1,4 @@
|
||||||
<div class="team-view-add-element">
|
<div class="team-view-add-element" focusable-popover-content>
|
||||||
<div class="entity-search"
|
<div class="entity-search"
|
||||||
namespace="orgname" placeholder="'Add a registered user or robot...'"
|
namespace="orgname" placeholder="'Add a registered user or robot...'"
|
||||||
entity-selected="addNewMember(entity)"
|
entity-selected="addNewMember(entity)"
|
||||||
|
|
|
@ -24,7 +24,7 @@
|
||||||
</div>
|
</div>
|
||||||
<div id="collapseRegister" class="panel-collapse collapse" ng-class="hasSignedIn() ? 'out' : 'in'">
|
<div id="collapseRegister" class="panel-collapse collapse" ng-class="hasSignedIn() ? 'out' : 'in'">
|
||||||
<div class="panel-body">
|
<div class="panel-body">
|
||||||
<div class="signup-form"></div>
|
<div class="signup-form" user-registered="handleUserRegistered(username)" invite-code="inviteCode"></div>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|
|
@ -1843,7 +1843,7 @@ quayApp = angular.module('quay', quayDependencies, function($provide, cfpLoading
|
||||||
when('/tour/features', {title: title + ' Features', templateUrl: '/static/partials/tour.html', controller: TourCtrl}).
|
when('/tour/features', {title: title + ' Features', templateUrl: '/static/partials/tour.html', controller: TourCtrl}).
|
||||||
when('/tour/enterprise', {title: 'Enterprise Edition', templateUrl: '/static/partials/tour.html', controller: TourCtrl}).
|
when('/tour/enterprise', {title: 'Enterprise Edition', templateUrl: '/static/partials/tour.html', controller: TourCtrl}).
|
||||||
|
|
||||||
when('/confirminvite', {title: 'Confirm Team Invite', templateUrl: '/static/partials/confirm-team-invite.html', controller: ConfirmInviteCtrl, reloadOnSearch: false}).
|
when('/confirminvite', {title: 'Confirm Invite', templateUrl: '/static/partials/confirm-invite.html', controller: ConfirmInviteCtrl, reloadOnSearch: false}).
|
||||||
|
|
||||||
when('/', {title: 'Hosted Private Docker Registry', templateUrl: '/static/partials/landing.html', controller: LandingCtrl,
|
when('/', {title: 'Hosted Private Docker Registry', templateUrl: '/static/partials/landing.html', controller: LandingCtrl,
|
||||||
pageClass: 'landing-page'}).
|
pageClass: 'landing-page'}).
|
||||||
|
@ -2218,6 +2218,36 @@ quayApp.directive('repoBreadcrumb', function () {
|
||||||
return directiveDefinitionObject;
|
return directiveDefinitionObject;
|
||||||
});
|
});
|
||||||
|
|
||||||
|
quayApp.directive('focusablePopoverContent', ['$timeout', '$popover', function ($timeout, $popover) {
|
||||||
|
return {
|
||||||
|
restrict: "A",
|
||||||
|
link: function (scope, element, attrs) {
|
||||||
|
$body = $('body');
|
||||||
|
var hide = function() {
|
||||||
|
$body.off('click');
|
||||||
|
scope.$apply(function() {
|
||||||
|
scope.$hide();
|
||||||
|
});
|
||||||
|
};
|
||||||
|
|
||||||
|
scope.$on('$destroy', function() {
|
||||||
|
$body.off('click');
|
||||||
|
});
|
||||||
|
|
||||||
|
$timeout(function() {
|
||||||
|
$body.on('click', function(evt) {
|
||||||
|
var target = evt.target;
|
||||||
|
var isPanelMember = $(element).has(target).length > 0 || target == element;
|
||||||
|
if (!isPanelMember) {
|
||||||
|
hide();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
$(element).find('input').focus();
|
||||||
|
}, 100);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
}]);
|
||||||
|
|
||||||
quayApp.directive('repoCircle', function () {
|
quayApp.directive('repoCircle', function () {
|
||||||
var directiveDefinitionObject = {
|
var directiveDefinitionObject = {
|
||||||
|
@ -2276,8 +2306,12 @@ quayApp.directive('userSetup', function () {
|
||||||
restrict: 'C',
|
restrict: 'C',
|
||||||
scope: {
|
scope: {
|
||||||
'redirectUrl': '=redirectUrl',
|
'redirectUrl': '=redirectUrl',
|
||||||
|
|
||||||
|
'inviteCode': '=inviteCode',
|
||||||
|
|
||||||
'signInStarted': '&signInStarted',
|
'signInStarted': '&signInStarted',
|
||||||
'signedIn': '&signedIn'
|
'signedIn': '&signedIn',
|
||||||
|
'userRegistered': '&userRegistered'
|
||||||
},
|
},
|
||||||
controller: function($scope, $location, $timeout, ApiService, KeyService, UserService) {
|
controller: function($scope, $location, $timeout, ApiService, KeyService, UserService) {
|
||||||
$scope.sendRecovery = function() {
|
$scope.sendRecovery = function() {
|
||||||
|
@ -2292,6 +2326,10 @@ quayApp.directive('userSetup', function () {
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
|
$scope.handleUserRegistered = function(username) {
|
||||||
|
$scope.userRegistered({'username': username});
|
||||||
|
};
|
||||||
|
|
||||||
$scope.hasSignedIn = function() {
|
$scope.hasSignedIn = function() {
|
||||||
return UserService.hasEverLoggedIn();
|
return UserService.hasEverLoggedIn();
|
||||||
};
|
};
|
||||||
|
@ -2390,7 +2428,9 @@ quayApp.directive('signupForm', function () {
|
||||||
transclude: true,
|
transclude: true,
|
||||||
restrict: 'C',
|
restrict: 'C',
|
||||||
scope: {
|
scope: {
|
||||||
|
'inviteCode': '=inviteCode',
|
||||||
|
|
||||||
|
'userRegistered': '&userRegistered'
|
||||||
},
|
},
|
||||||
controller: function($scope, $location, $timeout, ApiService, KeyService, UserService, Config, UIService) {
|
controller: function($scope, $location, $timeout, ApiService, KeyService, UserService, Config, UIService) {
|
||||||
$('.form-signup').popover();
|
$('.form-signup').popover();
|
||||||
|
@ -2411,6 +2451,10 @@ quayApp.directive('signupForm', function () {
|
||||||
UIService.hidePopover('#signupButton');
|
UIService.hidePopover('#signupButton');
|
||||||
$scope.registering = true;
|
$scope.registering = true;
|
||||||
|
|
||||||
|
if ($scope.inviteCode) {
|
||||||
|
$scope.newUser['inviteCode'] = $scope.inviteCode;
|
||||||
|
}
|
||||||
|
|
||||||
ApiService.createNewUser($scope.newUser).then(function() {
|
ApiService.createNewUser($scope.newUser).then(function() {
|
||||||
$scope.registering = false;
|
$scope.registering = false;
|
||||||
$scope.awaitingConfirmation = true;
|
$scope.awaitingConfirmation = true;
|
||||||
|
@ -2418,6 +2462,8 @@ quayApp.directive('signupForm', function () {
|
||||||
if (Config.MIXPANEL_KEY) {
|
if (Config.MIXPANEL_KEY) {
|
||||||
mixpanel.alias($scope.newUser.username);
|
mixpanel.alias($scope.newUser.username);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$scope.userRegistered({'username': $scope.newUser.username});
|
||||||
}, function(result) {
|
}, function(result) {
|
||||||
$scope.registering = false;
|
$scope.registering = false;
|
||||||
UIService.showFormError('#signupButton', result);
|
UIService.showFormError('#signupButton', result);
|
||||||
|
|
|
@ -2276,7 +2276,7 @@ function OrgAdminCtrl($rootScope, $scope, $timeout, Restangular, $routeParams, U
|
||||||
loadOrganization();
|
loadOrganization();
|
||||||
}
|
}
|
||||||
|
|
||||||
function TeamViewCtrl($rootScope, $scope, Restangular, ApiService, $routeParams) {
|
function TeamViewCtrl($rootScope, $scope, $timeout, Restangular, ApiService, $routeParams) {
|
||||||
var teamname = $routeParams.teamname;
|
var teamname = $routeParams.teamname;
|
||||||
var orgname = $routeParams.orgname;
|
var orgname = $routeParams.orgname;
|
||||||
|
|
||||||
|
@ -2755,6 +2755,7 @@ function TourCtrl($scope, $location) {
|
||||||
function ConfirmInviteCtrl($scope, $location, UserService, ApiService, NotificationService) {
|
function ConfirmInviteCtrl($scope, $location, UserService, ApiService, NotificationService) {
|
||||||
// Monitor any user changes and place the current user into the scope.
|
// Monitor any user changes and place the current user into the scope.
|
||||||
$scope.loading = false;
|
$scope.loading = false;
|
||||||
|
$scope.inviteCode = $location.search()['code'] || '';
|
||||||
|
|
||||||
UserService.updateUserIn($scope, function(user) {
|
UserService.updateUserIn($scope, function(user) {
|
||||||
if (!user.anonymous && !$scope.loading) {
|
if (!user.anonymous && !$scope.loading) {
|
||||||
|
|
|
@ -1,8 +1,10 @@
|
||||||
<div class="confirm-team-invite">
|
<div class="confirm-invite">
|
||||||
<div class="container signin-container">
|
<div class="container signin-container">
|
||||||
<div class="row">
|
<div class="row">
|
||||||
<div class="col-sm-6 col-sm-offset-3">
|
<div class="col-sm-6 col-sm-offset-3">
|
||||||
<div class="user-setup" ng-show="user.anonymous" redirect-url="redirectUrl"></div>
|
<div class="user-setup" ng-show="user.anonymous" redirect-url="redirectUrl"
|
||||||
|
invite-code="inviteCode">
|
||||||
|
</div>
|
||||||
<div class="quay-spinner" ng-show="!user.anonymous && loading"></div>
|
<div class="quay-spinner" ng-show="!user.anonymous && loading"></div>
|
||||||
<div class="alert alert-danger" ng-show="!user.anonymous && invalid">
|
<div class="alert alert-danger" ng-show="!user.anonymous && invalid">
|
||||||
Invalid confirmation code
|
Invalid confirmation code
|
|
@ -3,11 +3,12 @@
|
||||||
<div class="organization-header" organization="organization" team-name="teamname">
|
<div class="organization-header" organization="organization" team-name="teamname">
|
||||||
<div ng-show="canEditMembers" class="side-controls">
|
<div ng-show="canEditMembers" class="side-controls">
|
||||||
<div class="hidden-sm hidden-xs">
|
<div class="hidden-sm hidden-xs">
|
||||||
<button class="btn btn-success" data-trigger="click"
|
<button class="btn btn-success"
|
||||||
|
id="showAddMember"
|
||||||
data-title="Add Team Member"
|
data-title="Add Team Member"
|
||||||
data-content-template="/static/directives/team-view-add.html"
|
data-content-template="/static/directives/team-view-add.html"
|
||||||
data-placement="bottom-right"
|
data-placement="bottom-right"
|
||||||
bs-popover>
|
bs-popover="bs-popover">
|
||||||
<i class="fa fa-plus"></i>
|
<i class="fa fa-plus"></i>
|
||||||
Add Team Member
|
Add Team Member
|
||||||
</button>
|
</button>
|
||||||
|
|
|
@ -166,6 +166,13 @@ class ApiTestCase(unittest.TestCase):
|
||||||
parsed = py_json.loads(data)
|
parsed = py_json.loads(data)
|
||||||
return parsed
|
return parsed
|
||||||
|
|
||||||
|
def assertInTeam(self, data, membername):
|
||||||
|
for memberData in data['members']:
|
||||||
|
if memberData['name'] == membername:
|
||||||
|
return
|
||||||
|
|
||||||
|
self.fail(membername + ' not found in team: ' + json.dumps(data))
|
||||||
|
|
||||||
def login(self, username, password='password'):
|
def login(self, username, password='password'):
|
||||||
return self.postJsonResponse(Signin, data=dict(username=username, password=password))
|
return self.postJsonResponse(Signin, data=dict(username=username, password=password))
|
||||||
|
|
||||||
|
@ -378,6 +385,28 @@ class TestCreateNewUser(ApiTestCase):
|
||||||
expected_code=201)
|
expected_code=201)
|
||||||
self.assertEquals('"Created"', data)
|
self.assertEquals('"Created"', data)
|
||||||
|
|
||||||
|
def test_createuser_withteaminvite(self):
|
||||||
|
inviter = model.get_user(ADMIN_ACCESS_USER)
|
||||||
|
team = model.get_organization_team(ORGANIZATION, 'owners')
|
||||||
|
invite = model.add_or_invite_to_team(inviter, team, None, 'foo@example.com')
|
||||||
|
|
||||||
|
details = {
|
||||||
|
'inviteCode': invite.invite_token
|
||||||
|
}
|
||||||
|
details.update(NEW_USER_DETAILS);
|
||||||
|
|
||||||
|
data = self.postResponse(User,
|
||||||
|
data=details,
|
||||||
|
expected_code=201)
|
||||||
|
self.assertEquals('"Created"', data)
|
||||||
|
|
||||||
|
# Make sure the user was added to the team.
|
||||||
|
self.login(ADMIN_ACCESS_USER)
|
||||||
|
json = self.getJsonResponse(TeamMemberList,
|
||||||
|
params=dict(orgname=ORGANIZATION,
|
||||||
|
teamname='owners'))
|
||||||
|
self.assertInTeam(json, NEW_USER_DETAILS['username'])
|
||||||
|
|
||||||
|
|
||||||
class TestSignout(ApiTestCase):
|
class TestSignout(ApiTestCase):
|
||||||
def test_signout(self):
|
def test_signout(self):
|
||||||
|
@ -743,13 +772,6 @@ class TestGetOrganizationTeamMembers(ApiTestCase):
|
||||||
|
|
||||||
|
|
||||||
class TestUpdateOrganizationTeamMember(ApiTestCase):
|
class TestUpdateOrganizationTeamMember(ApiTestCase):
|
||||||
def assertInTeam(self, data, membername):
|
|
||||||
for memberData in data['members']:
|
|
||||||
if memberData['name'] == membername:
|
|
||||||
return
|
|
||||||
|
|
||||||
self.fail(membername + ' not found in team: ' + json.dumps(data))
|
|
||||||
|
|
||||||
def test_addmember_alreadyteammember(self):
|
def test_addmember_alreadyteammember(self):
|
||||||
self.login(ADMIN_ACCESS_USER)
|
self.login(ADMIN_ACCESS_USER)
|
||||||
|
|
||||||
|
|
|
@ -34,6 +34,27 @@ def parse_robot_username(robot_username):
|
||||||
return robot_username.split('+', 2)
|
return robot_username.split('+', 2)
|
||||||
|
|
||||||
|
|
||||||
|
def parse_urn(urn):
|
||||||
|
""" Parses a URN, returning a pair that contains a list of URN
|
||||||
|
namespace parts, followed by the URN's unique ID.
|
||||||
|
"""
|
||||||
|
if not urn.startswith('urn:'):
|
||||||
|
return None
|
||||||
|
|
||||||
|
parts = urn[len('urn:'):].split(':')
|
||||||
|
return (parts[0:len(parts) - 1], parts[len(parts) - 1])
|
||||||
|
|
||||||
|
|
||||||
|
def parse_single_urn(urn):
|
||||||
|
""" Parses a URN, returning a pair that contains the first
|
||||||
|
namespace part, followed by the URN's unique ID.
|
||||||
|
"""
|
||||||
|
result = parse_urn(urn)
|
||||||
|
if result is None or not len(result[0]):
|
||||||
|
return None
|
||||||
|
|
||||||
|
return (result[0][0], result[1])
|
||||||
|
|
||||||
uuid_generator = lambda: str(uuid4())
|
uuid_generator = lambda: str(uuid4())
|
||||||
|
|
||||||
|
|
||||||
|
|
Reference in a new issue