Skip to content

Commit d9c958a

Browse files
nickstenningnickstenning
committed
Look up {identity,authorization}Policy in the registry
Extension modules should look up implementations of particular interfaces in the application registry, not by magic keyword on the application itself. This will give better error messages if a component is missing.
1 parent aa65bb2 commit d9c958a

File tree

4 files changed

+52
-40
lines changed

4 files changed

+52
-40
lines changed

‎src/ui/main.js‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,12 @@ function removeDynamicStyle() {
106106

107107

108108
// Helper function to add permissions checkboxes to the editor
109-
function addPermissionsCheckboxes(editor, app) {
109+
function addPermissionsCheckboxes(editor, ident, authz) {
110110
function createLoadCallback(action) {
111111
return function loadCallback(field, annotation) {
112112
field = util.$(field).show();
113113

114-
var u = app.ident.who();
114+
var u = ident.who();
115115
var input = field.find('input');
116116

117117
// Do not show field if no user is set
@@ -120,12 +120,12 @@ function addPermissionsCheckboxes(editor, app) {
120120
}
121121

122122
// Do not show field if current user is not admin.
123-
if (!(app.authz.permits('admin', annotation, u))) {
123+
if (!(authz.permits('admin', annotation, u))) {
124124
field.hide();
125125
}
126126

127127
// See if we can authorise without a user.
128-
if (app.authz.permits(action, annotation, null)) {
128+
if (authz.permits(action, annotation, null)) {
129129
input.attr('checked', 'checked');
130130
} else {
131131
input.removeAttr('checked');
@@ -135,7 +135,7 @@ function addPermissionsCheckboxes(editor, app) {
135135

136136
function createSubmitCallback(action) {
137137
return function submitCallback(field, annotation) {
138-
var u = app.ident.who();
138+
var u = ident.who();
139139

140140
// Don't do anything if no user is set
141141
if (typeof u === 'undefined' || u === null) {
@@ -153,7 +153,7 @@ function addPermissionsCheckboxes(editor, app) {
153153
// interpret "prevent others from viewing" as meaning "allow
154154
// only me to view". This may want changing in the future.
155155
annotation.permissions[action] = [
156-
app.authz.authorizedUserId(u)
156+
authz.authorizedUserId(u)
157157
];
158158
}
159159
};
@@ -224,6 +224,9 @@ function main(options) {
224224
};
225225

226226
function start(app) {
227+
var ident = app.registry.getUtility('identityPolicy');
228+
var authz = app.registry.getUtility('authorizationPolicy');
229+
227230
s.adder = new adder.Adder({
228231
onCreate: function (ann) {
229232
app.annotations.create(ann);
@@ -236,7 +239,7 @@ function main(options) {
236239
});
237240
s.editor.attach();
238241

239-
addPermissionsCheckboxes(s.editor, app);
242+
addPermissionsCheckboxes(s.editor, ident, authz);
240243

241244
s.highlighter = new highlighter.Highlighter(options.element);
242245

@@ -264,10 +267,10 @@ function main(options) {
264267
app.annotations['delete'](ann);
265268
},
266269
permitEdit: function (ann) {
267-
return app.authz.permits('update', ann, app.ident.who());
270+
return authz.permits('update', ann, ident.who());
268271
},
269272
permitDelete: function (ann) {
270-
return app.authz.permits('delete', ann, app.ident.who());
273+
return authz.permits('delete', ann, ident.who());
271274
},
272275
autoViewHighlights: options.element,
273276
extensions: options.viewerExtensions

‎src/ui/viewer.js‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,9 @@ exports.standalone = function standalone(options) {
507507

508508
return {
509509
start: function (app) {
510+
var ident = app.registry.getUtility('identityPolicy');
511+
var authz = app.registry.getUtility('authorizationPolicy');
512+
510513
// Set default handlers for what happens when the user clicks the
511514
// edit and delete buttons:
512515
if (typeof options.onEdit === 'undefined') {
@@ -524,20 +527,12 @@ exports.standalone = function standalone(options) {
524527
// buttons are shown in the viewer:
525528
if (typeof options.permitEdit === 'undefined') {
526529
options.permitEdit = function (annotation) {
527-
return app.authz.permits(
528-
'update',
529-
annotation,
530-
app.ident.who()
531-
);
530+
return authz.permits('update', annotation, ident.who());
532531
};
533532
}
534533
if (typeof options.permitDelete === 'undefined') {
535534
options.permitDelete = function (annotation) {
536-
return app.authz.permits(
537-
'delete',
538-
annotation,
539-
app.ident.who()
540-
);
535+
return authz.permits('delete', annotation, ident.who());
541536
};
542537
}
543538

‎test/spec/ui/main_spec.js‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,25 @@ var main = require('../../../src/ui/main').main;
1515

1616
describe('annotator.ui.main', function () {
1717
var sandbox;
18+
var mockAuthz;
19+
var mockIdent;
1820
var mockApp;
1921

2022
beforeEach(function () {
2123
sandbox = sinon.sandbox.create();
24+
mockAuthz = {
25+
permits: sandbox.stub().returns(true),
26+
authorizedUserId: function (u) { return u; }
27+
};
28+
mockIdent = {who: sandbox.stub().returns('alice')};
2229
mockApp = {
2330
annotations: {create: sandbox.stub()},
24-
authz: {
25-
permits: sandbox.stub().returns(true),
26-
authorizedUserId: function (u) { return u; }
27-
},
28-
ident: {who: sandbox.stub().returns('alice')}
31+
registry: {
32+
getUtility: sandbox.stub()
33+
}
2934
};
35+
mockApp.registry.getUtility.withArgs('authorizationPolicy').returns(mockAuthz);
36+
mockApp.registry.getUtility.withArgs('identityPolicy').returns(mockIdent);
3037
});
3138

3239
afterEach(function () {
@@ -155,37 +162,37 @@ describe('annotator.ui.main', function () {
155162
});
156163

157164
it("load hides a field if no user is set", function () {
158-
mockApp.ident.who.returns(null);
165+
mockIdent.who.returns(null);
159166
viewLoad(field, {});
160167
assert.equal(field.style.display, 'none');
161168
});
162169

163170
it("load hides a field if current user is not admin", function () {
164171
var ann = {};
165-
mockApp.authz.permits
172+
mockAuthz.permits
166173
.withArgs('admin', ann, 'alice').returns(false);
167174
viewLoad(field, ann);
168175
assert.equal(field.style.display, 'none');
169176
});
170177

171178
it("load hides a field if current user is not admin", function () {
172179
var ann = {};
173-
mockApp.authz.permits
180+
mockAuthz.permits
174181
.withArgs('admin', ann, 'alice').returns(false);
175182
viewLoad(field, ann);
176183
assert.equal(field.style.display, 'none');
177184
});
178185

179186
it("load shows a checked field if the action is authorised with a null user", function () {
180-
mockApp.authz.permits.returns(true);
187+
mockAuthz.permits.returns(true);
181188
viewLoad(field, {});
182189
assert.notEqual(field.style.display, 'none');
183190
assert.isTrue(field.firstChild.checked);
184191
});
185192

186193
it("load shows an unchecked field if the action isn't authorised with a null user", function () {
187194
var ann = {};
188-
mockApp.authz.permits
195+
mockAuthz.permits
189196
.withArgs('read', ann, null).returns(false);
190197
viewLoad(field, ann);
191198
assert.notEqual(field.style.display, 'none');
@@ -209,7 +216,7 @@ describe('annotator.ui.main', function () {
209216
});
210217

211218
it("submit doesn't touch the annotation if the current user is null", function () {
212-
mockApp.ident.who.returns(null);
219+
mockIdent.who.returns(null);
213220
var ann = {permissions: {'read': ['alice']}};
214221
field.firstChild.checked = true;
215222
viewSubmit(field, ann);

‎test/spec/ui/viewer_spec.js‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -370,24 +370,31 @@ describe('ui.viewer.Viewer', function () {
370370

371371

372372
describe('ui.viewer.standalone', function () {
373-
var mockApp = null,
374-
mockViewer = null,
375-
sandbox = null;
373+
var sandbox;
374+
var mockAuthz;
375+
var mockIdent;
376+
var mockApp;
377+
var mockViewer;
376378

377379
beforeEach(function () {
378380
sandbox = sinon.sandbox.create();
381+
mockAuthz = {
382+
permits: sandbox.stub()
383+
};
384+
mockIdent = {
385+
who: sandbox.stub().returns('alice')
386+
};
379387
mockApp = {
380388
annotations: {
381389
update: sandbox.stub(),
382390
"delete": sandbox.stub()
383391
},
384-
authz: {
385-
permits: sandbox.stub()
386-
},
387-
ident: {
388-
who: sandbox.stub().returns('alice')
392+
registry: {
393+
getUtility: sandbox.stub()
389394
}
390395
};
396+
mockApp.registry.getUtility.withArgs('authorizationPolicy').returns(mockAuthz);
397+
mockApp.registry.getUtility.withArgs('identityPolicy').returns(mockIdent);
391398
mockViewer = {
392399
destroy: sandbox.stub()
393400
};
@@ -428,7 +435,7 @@ describe('ui.viewer.standalone', function () {
428435
assert(sinon.match.has('permitEdit').test(passedOptions));
429436
passedOptions.permitEdit({text: 'foo'});
430437
sinon.assert.calledWith(
431-
mockApp.authz.permits,
438+
mockAuthz.permits,
432439
'update',
433440
{text: 'foo'},
434441
'alice'
@@ -441,7 +448,7 @@ describe('ui.viewer.standalone', function () {
441448
assert(sinon.match.has('permitDelete').test(passedOptions));
442449
passedOptions.permitDelete({text: 'foo'});
443450
sinon.assert.calledWith(
444-
mockApp.authz.permits,
451+
mockAuthz.permits,
445452
'delete',
446453
{text: 'foo'},
447454
'alice'

0 commit comments

Comments
 (0)