Skip to content

Commit 0102c49

Browse files
author
OpenShift Bot
authored
Merge pull request #1113 from benjaminapetersen/issue-1103-membership-refresh-when-losing-admin
Merged by openshift-bot
2 parents fa6f2af + 9573cce commit 0102c49

File tree

6 files changed

+107
-64
lines changed

6 files changed

+107
-64
lines changed

app/scripts/controllers/membership.js

+25-3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ angular
2222
var projectName = $routeParams.project;
2323
var humanizeKind = $filter('humanizeKind');
2424
var annotation = $filter('annotation');
25+
var canI = $filter('canI');
2526

2627
var allRoles = [];
2728

@@ -69,7 +70,7 @@ angular
6970
$scope.newBinding.newRole = null;
7071
};
7172

72-
var refreshRoleBindingList = function() {
73+
var refreshRoleBindingList = function(toUpdateOnError) {
7374
DataService
7475
.list('rolebindings', requestContext, null , {
7576
errorNotification: false
@@ -81,6 +82,13 @@ angular
8182
subjectKindsForUI: MembershipService.mapRolebindingsForUI(resp.by('metadata.name'), allRoles)
8283
});
8384
resetForm();
85+
}, function() {
86+
// if the request errors but we have an object, we can at least update in place
87+
if(toUpdateOnError) {
88+
$scope.roleBindings[toUpdateOnError.metadata.name] = toUpdateOnError;
89+
$scope.subjectKindsForUI = MembershipService.mapRolebindingsForUI($scope.roleBindings, allRoles);
90+
}
91+
resetForm();
8492
});
8593
};
8694

@@ -238,6 +246,7 @@ angular
238246
angular.extend($scope, {
239247
project: project,
240248
subjectKinds: subjectKinds,
249+
canUpdateRolebindings: canI('rolebindings', 'update', projectName),
241250
confirmRemove: function(subjectName, kindName, roleName) {
242251
var redirectToProjectList = null;
243252
var modalScope = createModalScope(subjectName, kindName, roleName, $scope.user.metadata.name);
@@ -259,11 +268,24 @@ angular
259268
.result.then(function() {
260269
RoleBindingsService
261270
.removeSubject(subjectName, roleName, $scope.roleBindings, requestContext)
262-
.then(function() {
271+
.then(function(updateRolebinding) {
263272
if(redirectToProjectList) {
264273
$location.url("./");
265274
} else {
266-
refreshRoleBindingList();
275+
AuthorizationService
276+
.getProjectRules(projectName, true)
277+
.then(function() {
278+
refreshRoleBindingList(updateRolebinding[0]);
279+
// test if the current user can still edit roles.
280+
// if not, remove permissions & exit edit mode.
281+
var canEdit = canI('rolebindings', 'update', projectName);
282+
angular.extend($scope, {
283+
canUpdateRolebindings: canEdit,
284+
mode: {
285+
edit: $scope.mode.edit ? canEdit : false
286+
}
287+
});
288+
});
267289
showAlert('rolebindingUpdate', 'success', messages.remove.success({
268290
roleName: roleName,
269291
subjectName: subjectName

app/scripts/services/authorization.js

+6-5
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
angular.module("openshiftConsole")
44
.factory("AuthorizationService", function($q, $cacheFactory, Logger, $interval, APIService, DataService){
5-
5+
66
var currentProject = null;
77
var cachedRulesByProject = $cacheFactory('rulesCache', {
88
number: 10
@@ -34,7 +34,7 @@ angular.module("openshiftConsole")
3434
// Check if resource name meets one of following conditions, since those resources can't be create/update via `Add to project` page:
3535
// - 'projectrequests'
3636
// - subresource that contains '/', eg: 'builds/source', 'builds/logs', ...
37-
// - resource is in REVIEW_RESOURCES list
37+
// - resource is in REVIEW_RESOURCES list
3838
var checkResource = function(resource) {
3939
if (resource === "projectrequests" || _.contains(resource, "/") || _.contains(REVIEW_RESOURCES, resource)) {
4040
return false;
@@ -52,12 +52,13 @@ angular.module("openshiftConsole")
5252
});
5353
};
5454

55-
var getProjectRules = function(projectName) {
55+
// forceRefresh is a boolean to bust the cache & request new perms
56+
var getProjectRules = function(projectName, forceRefresh) {
5657
var deferred = $q.defer();
5758
currentProject = projectName;
5859
var projectRules = cachedRulesByProject.get(projectName);
5960
var rulesResource = "selfsubjectrulesreviews";
60-
if (!projectRules || projectRules.forceRefresh) {
61+
if (!projectRules || projectRules.forceRefresh || forceRefresh) {
6162
// Check if APIserver contains 'selfsubjectrulesreviews' resource. If not switch to permissive mode.
6263
if (APIService.apiInfo(rulesResource)) {
6364
Logger.log("AuthorizationService, loading user rules for " + projectName + " project");
@@ -127,7 +128,7 @@ angular.module("openshiftConsole")
127128
_canI(rules, verb, '*', '*' ) ||
128129
_canI(rules, verb, r.group, '*' ) ||
129130
_canI(rules, verb, '*', r.resource);
130-
};
131+
};
131132

132133
var canIAddToProject = function(projectName) {
133134
if (permissiveMode) {

app/scripts/services/membership/roleBindings.js

+8-2
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,14 @@ angular
9696
cleanBinding(binding);
9797
binding.subjects = _.reject(binding.subjects, {name: subjectName});
9898
return binding.subjects.length ?
99-
DataService.update('rolebindings', binding.metadata.name, binding, context):
100-
DataService.delete('rolebindings', binding.metadata.name, context);
99+
DataService.update('rolebindings', binding.metadata.name, binding, context) :
100+
DataService.delete('rolebindings', binding.metadata.name, context)
101+
// For a delete, resp is simply a 201 or less useful object.
102+
// Instead, this intercepts the response & returns the binding object
103+
// with the empty .subjects[] list.
104+
.then(function() {
105+
return binding;
106+
});
101107
}));
102108
};
103109

app/views/membership.html

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ <h1>
1010
<a
1111
class="pull-right btn btn-default"
1212
href=""
13-
ng-if="'rolebindings' | canI : 'update'"
13+
ng-if="canUpdateRolebindings"
1414
ng-click="toggleEditMode()">
1515
<span ng-if="!(mode.edit)">Edit Membership</span>
1616
<span ng-if="mode.edit">Done Editing</span>

dist/scripts/scripts.js

+66-52
Original file line numberDiff line numberDiff line change
@@ -1072,31 +1072,31 @@ return _.some(a.resources, function(b) {
10721072
return l(b) && !_.isEmpty(_.intersection(a.verbs, [ "*", "create", "update" ]));
10731073
});
10741074
});
1075-
}, n = function(b) {
1076-
var d = a.defer();
1075+
}, n = function(b, d) {
1076+
var j = a.defer();
10771077
g = b;
1078-
var j = h.get(b), l = "selfsubjectrulesreviews";
1079-
if (!j || j.forceRefresh) if (e.apiInfo(l)) {
1078+
var l = h.get(b), n = "selfsubjectrulesreviews";
1079+
if (!l || l.forceRefresh || d) if (e.apiInfo(n)) {
10801080
c.log("AuthorizationService, loading user rules for " + b + " project");
1081-
var n = {
1081+
var o = {
10821082
kind:"SelfSubjectRulesReview",
10831083
apiVersion:"v1"
10841084
};
1085-
f.create(l, null, n, {
1085+
f.create(n, null, o, {
10861086
namespace:b
10871087
}).then(function(a) {
1088-
var c = k(a.status.rules), e = m(a.status.rules);
1088+
var c = k(a.status.rules), d = m(a.status.rules);
10891089
h.put(b, {
10901090
rules:c,
1091-
canAddToProject:e,
1091+
canAddToProject:d,
10921092
forceRefresh:!1,
10931093
cacheTimestamp:_.now()
1094-
}), d.resolve();
1094+
}), j.resolve();
10951095
}, function() {
1096-
i = !0, d.resolve();
1096+
i = !0, j.resolve();
10971097
});
1098-
} else c.log("AuthorizationService, resource 'selfsubjectrulesreviews' is not part of APIserver. Switching into permissive mode."), i = !0, d.resolve(); else c.log("AuthorizationService, using cached rules for " + b + " project"), _.now() - j.cacheTimestamp >= 6e5 && (j.forceRefresh = !0), d.resolve();
1099-
return d.promise;
1098+
} else c.log("AuthorizationService, resource 'selfsubjectrulesreviews' is not part of APIserver. Switching into permissive mode."), i = !0, j.resolve(); else c.log("AuthorizationService, using cached rules for " + b + " project"), _.now() - l.cacheTimestamp >= 6e5 && (l.forceRefresh = !0), j.resolve();
1099+
return j.promise;
11001100
}, o = function(a) {
11011101
return _.get(h.get(a || g), [ "rules" ]);
11021102
}, p = function(a, b, c, d) {
@@ -2892,7 +2892,9 @@ return a.all(_.map(i, function(a) {
28922892
var d = e();
28932893
return a = _.extend(d, a), g(a), a.subjects = _.reject(a.subjects, {
28942894
name:c
2895-
}), a.subjects.length ? b.update("rolebindings", a.metadata.name, a, h) :b["delete"]("rolebindings", a.metadata.name, h);
2895+
}), a.subjects.length ? b.update("rolebindings", a.metadata.name, a, h) :b["delete"]("rolebindings", a.metadata.name, h).then(function() {
2896+
return a;
2897+
});
28962898
}));
28972899
}, k = function(a, d, e) {
28982900
return b.list("rolebindings", a, function(a) {
@@ -5178,7 +5180,7 @@ a !== b && (localStorage.setItem("monitoring.eventsidebar.collapsed", c.renderOp
51785180
});
51795181
}));
51805182
} ]), angular.module("openshiftConsole").controller("MembershipController", [ "$filter", "$location", "$routeParams", "$scope", "$timeout", "$uibModal", "AuthService", "AuthorizationService", "DataService", "ProjectsService", "MembershipService", "RoleBindingsService", "RolesService", function(a, b, c, d, e, f, g, h, i, j, k, l, m) {
5181-
var n, o = c.project, p = a("humanizeKind"), q = a("annotation"), r = [], s = {
5183+
var n, o = c.project, p = a("humanizeKind"), q = a("annotation"), r = a("canI"), s = [], t = {
51825184
notice:{
51835185
yourLastRole:_.template('Removing the role "<%= roleName %>" may completely remove your ability to see this project.')
51845186
},
@@ -5203,69 +5205,71 @@ exists:_.template('The role "<%= roleName %>" has already been granted to "<%= s
52035205
}
52045206
},
52055207
errorReason:_.template('Reason: "<%= httpErr %>"')
5206-
}, t = function(a, b, c, e, f) {
5208+
}, u = function(a, b, c, e, f) {
52075209
f = f || d, f.alerts[a] = {
52085210
type:b,
52095211
message:c,
52105212
details:e
52115213
};
5212-
}, u = function() {
5213-
d.disableAddForm = !1, d.newBinding.name = "", d.newBinding.namespace = o, d.newBinding.newRole = null;
52145214
}, v = function() {
5215+
d.disableAddForm = !1, d.newBinding.name = "", d.newBinding.namespace = o, d.newBinding.newRole = null;
5216+
}, w = function(a) {
52155217
i.list("rolebindings", n, null, {
52165218
errorNotification:!1
52175219
}).then(function(a) {
52185220
angular.extend(d, {
52195221
canShowRoles:!0,
52205222
roleBindings:a.by("metadata.name"),
5221-
subjectKindsForUI:k.mapRolebindingsForUI(a.by("metadata.name"), r)
5222-
}), u();
5223+
subjectKindsForUI:k.mapRolebindingsForUI(a.by("metadata.name"), s)
5224+
}), v();
5225+
}, function() {
5226+
a && (d.roleBindings[a.metadata.name] = a, d.subjectKindsForUI = k.mapRolebindingsForUI(d.roleBindings, s)), v();
52235227
});
5224-
}, w = function(b, c) {
5228+
}, x = function(b, c) {
52255229
d.disableAddForm = !0, l.create(b, c, o, n).then(function() {
5226-
v(), t("rolebindingCreate", "success", s.update.subject.success({
5230+
w(), u("rolebindingCreate", "success", t.update.subject.success({
52275231
roleName:b.metadata.name,
52285232
subjectName:c.name
52295233
}));
52305234
}, function(d) {
5231-
u(), t("rolebindingCreateFail", "error", s.update.subject.error({
5235+
v(), u("rolebindingCreateFail", "error", t.update.subject.error({
52325236
roleName:b.metadata.name,
52335237
subjectName:c.name
5234-
}), s.errorReason({
5238+
}), t.errorReason({
52355239
httpErr:a("getErrorDetails")(d)
52365240
}));
52375241
});
5238-
}, x = function(b, c, e) {
5242+
}, y = function(b, c, e) {
52395243
d.disableAddForm = !0, l.addSubject(b, c, e, n).then(function() {
5240-
v(), t("rolebindingUpdate", "success", s.update.subject.success({
5244+
w(), u("rolebindingUpdate", "success", t.update.subject.success({
52415245
roleName:b.roleRef.name,
52425246
subjectName:c.name
52435247
}));
52445248
}, function(d) {
5245-
u(), t("rolebindingUpdateFail", "error", s.update.subject.error({
5249+
v(), u("rolebindingUpdateFail", "error", t.update.subject.error({
52465250
roleName:b.roleRef.name,
52475251
subjectName:c.name
5248-
}), s.errorReason({
5252+
}), t.errorReason({
52495253
httpErr:a("getErrorDetails")(d)
52505254
}));
52515255
});
5252-
}, y = {};
5253-
c.tab && (y[c.tab] = !0);
5254-
var z = k.getSubjectKinds();
5256+
}, z = {};
5257+
c.tab && (z[c.tab] = !0);
5258+
var A = k.getSubjectKinds();
52555259
angular.extend(d, {
5256-
selectedTab:y,
5260+
selectedTab:z,
52575261
projectName:o,
52585262
alerts:{},
52595263
forms:{},
52605264
emptyMessage:"Loading...",
5261-
subjectKinds:z,
5265+
subjectKinds:A,
52625266
newBinding:{
52635267
role:"",
52645268
kind:c.tab || "User",
52655269
name:""
52665270
},
52675271
toggleEditMode:function() {
5268-
u(), d.mode.edit = !d.mode.edit;
5272+
v(), d.mode.edit = !d.mode.edit;
52695273
},
52705274
mode:{
52715275
edit:!1
@@ -5291,10 +5295,10 @@ return a ? e + (q(a, "description") || b) :b;
52915295
}
52925296
}
52935297
});
5294-
var A = function(a, b, c, e) {
5298+
var B = function(a, b, c, e) {
52955299
var f = {
52965300
alerts:{},
5297-
detailsMarkup:s.remove.areYouSure.html.subject({
5301+
detailsMarkup:t.remove.areYouSure.html.subject({
52985302
roleName:c,
52995303
kindName:p(b),
53005304
subjectName:a
@@ -5303,12 +5307,12 @@ okButtonText:"Remove",
53035307
okButtonClass:"btn-danger",
53045308
cancelButtonText:"Cancel"
53055309
};
5306-
return _.isEqual(a, e) && (f.detailsMarkup = s.remove.areYouSure.html.self({
5310+
return _.isEqual(a, e) && (f.detailsMarkup = t.remove.areYouSure.html.self({
53075311
roleName:c,
53085312
subjectName:a
5309-
}), k.isLastRole(d.user.metadata.name, d.roleBindings) && t("currentUserLastRole", "error", s.notice.yourLastRole({
5313+
}), k.isLastRole(d.user.metadata.name, d.roleBindings) && u("currentUserLastRole", "error", t.notice.yourLastRole({
53105314
roleName:c
5311-
}), null, f)), _.isEqual(b, "ServiceAccount") && _.startsWith(c, "system:") && t("editingServiceAccountRole", "error", s.warning.serviceAccount(), null, f), f;
5315+
}), null, f)), _.isEqual(b, "ServiceAccount") && _.startsWith(c, "system:") && u("editingServiceAccountRole", "error", t.warning.serviceAccount(), null, f), f;
53125316
};
53135317
g.withUser().then(function(a) {
53145318
d.user = a;
@@ -5323,31 +5327,41 @@ a && !_.includes(d.projects, a) ? d.projects = [ a ].concat(b) :d.projects = b;
53235327
}
53245328
});
53255329
}), j.get(c.project).then(_.spread(function(c, e) {
5326-
n = e, v(), angular.extend(d, {
5330+
n = e, w(), angular.extend(d, {
53275331
project:c,
5328-
subjectKinds:z,
5332+
subjectKinds:A,
5333+
canUpdateRolebindings:r("rolebindings", "update", o),
53295334
confirmRemove:function(c, e, g) {
5330-
var h = null, i = A(c, e, g, d.user.metadata.name);
5331-
_.isEqual(c, d.user.metadata.name) && k.isLastRole(d.user.metadata.name, d.roleBindings) && (h = !0), f.open({
5335+
var i = null, j = B(c, e, g, d.user.metadata.name);
5336+
_.isEqual(c, d.user.metadata.name) && k.isLastRole(d.user.metadata.name, d.roleBindings) && (i = !0), f.open({
53325337
animation:!0,
53335338
templateUrl:"views/modals/confirm.html",
53345339
controller:"ConfirmModalController",
53355340
resolve:{
53365341
modalConfig:function() {
5337-
return i;
5342+
return j;
53385343
}
53395344
}
53405345
}).result.then(function() {
5341-
l.removeSubject(c, g, d.roleBindings, n).then(function() {
5342-
h ? b.url("./") :(v(), t("rolebindingUpdate", "success", s.remove.success({
5346+
l.removeSubject(c, g, d.roleBindings, n).then(function(a) {
5347+
i ? b.url("./") :(h.getProjectRules(o, !0).then(function() {
5348+
w(a[0]);
5349+
var b = r("rolebindings", "update", o);
5350+
angular.extend(d, {
5351+
canUpdateRolebindings:b,
5352+
mode:{
5353+
edit:!!d.mode.edit && b
5354+
}
5355+
});
5356+
}), u("rolebindingUpdate", "success", t.remove.success({
53435357
roleName:g,
53445358
subjectName:c
53455359
})));
53465360
}, function(b) {
5347-
t("rolebindingUpdateFail", "error", s.remove.error({
5361+
u("rolebindingUpdateFail", "error", t.remove.error({
53485362
roleName:g,
53495363
subjectName:c
5350-
}), s.errorReason({
5364+
}), t.errorReason({
53515365
httpErr:a("getErrorDetails")(b)
53525366
}));
53535367
});
@@ -5365,23 +5379,23 @@ name:c.metadata.name
53655379
});
53665380
g && _.some(g.subjects, {
53675381
name:a
5368-
}) ? t("rolebindingUpdate", "info", s.update.subject.exists({
5382+
}) ? u("rolebindingUpdate", "info", t.update.subject.exists({
53695383
roleName:c.metadata.name,
53705384
subjectName:a
5371-
})) :g ? x(g, f, e) :w(c, f, e);
5385+
})) :g ? y(g, f, e) :x(c, f, e);
53725386
}
53735387
}), m.listAllRoles(n, {
53745388
errorNotification:!1
53755389
}).then(function(a) {
5376-
r = k.mapRolesForUI(_.first(a).by("metadata.name"), _.last(a).by("metadata.name"));
5377-
var b = k.sortRoles(r), c = k.filterRoles(r), e = function(a, b) {
5390+
s = k.mapRolesForUI(_.first(a).by("metadata.name"), _.last(a).by("metadata.name"));
5391+
var b = k.sortRoles(s), c = k.filterRoles(s), e = function(a, b) {
53785392
return _.some(b, {
53795393
metadata:{
53805394
name:a
53815395
}
53825396
});
53835397
};
5384-
v(), angular.extend(d, {
5398+
w(), angular.extend(d, {
53855399
toggle:{
53865400
roles:!1
53875401
},

0 commit comments

Comments
 (0)