Hide the edit/add buttons for fields the user can't update

Currently the /registrar-settings backend endpoint will fail to update any
OWNER fields that a non-OWNER tries to change.

However, the front-end (soy, js) still allow non-OWNERs to try and change
these fields (there's the "edit" or "add" button, and it only fails when you try to "save")

This CL changes the front-end to remove the ability for non-OWNERs to even try
and change these fields. However, it will still let them *view* these fields as
it has interesting and important information.

-------------------------------

In addition - it changes the webdriver tests to include the "edit buttons". Those were never tested before, and now we will test to see if they are indeed displayed or not.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=223845883
This commit is contained in:
guyben 2018-12-03 12:51:37 -08:00 committed by jianglai
parent f980a25b32
commit a3a60075a0
19 changed files with 118 additions and 74 deletions

View file

@ -1247,7 +1247,7 @@ Component: App bars
position: relative; position: relative;
padding: 21px 0; padding: 21px 0;
border-bottom: 1px solid #ebebeb; border-bottom: 1px solid #ebebeb;
height: 29px; height: 66px;
z-index: 20; z-index: 20;
background: #fff; background: #fff;
} }
@ -1263,6 +1263,22 @@ Component: App bars
color: #666; color: #666;
white-space:nowrap; white-space:nowrap;
} }
.kd-appbar .kd-description {
height: 29px;
font-size: 20px;
font-weight: normal;
line-height: 29px;
padding-bottom: 8px;
color: #666;
white-space:nowrap;
}
.kd-appbar .kd-description .kd-value {
color: black;
font-weight: bold;
}
.kd-appbar .kd-description form {
display: inline;
}
.kd-appbar .kd-appname a { color: #666; cursor:pointer; } .kd-appbar .kd-appname a { color: #666; cursor:pointer; }
#stickers .kd-appbar .kd-buttonbar { #stickers .kd-appbar .kd-buttonbar {
margin-bottom: 0; margin-bottom: 0;

View file

@ -196,13 +196,10 @@ li.kd-menulistitem {
} }
.kd-appbar { .kd-appbar {
padding: 0.75em 0; /* same as in reg-content below. lines the left edge of the
} appbuttons and content area with the 'r' in registry. */
#reg-app-buttons {
/* Same as in reg-content below. Lines the left edge of the
appbuttons and content area with the 'R' in Registry. */
padding-left: 173px; padding-left: 173px;
padding-top: .75em;
} }
.kd-content-sidebar { .kd-content-sidebar {
@ -214,7 +211,7 @@ li.kd-menulistitem {
#reg-nav { #reg-nav {
position: fixed; position: fixed;
left: 0; left: 0;
top: 128px; top: 136px;
width: 155px; width: 155px;
margin: 0 25px 0 0; margin: 0 25px 0 0;
z-index: 3; z-index: 3;
@ -261,7 +258,7 @@ li.kd-menulistitem {
#reg-content-and-footer { #reg-content-and-footer {
position: absolute; position: absolute;
top: 105px; top: 136px;
left: 173px; left: 173px;
bottom: 0; bottom: 0;
width: 100%; width: 100%;

View file

@ -31,10 +31,11 @@ goog.forwardDeclare('registry.Console');
* An editable item, with Edit and Save/Cancel buttons in the appbar. * An editable item, with Edit and Save/Cancel buttons in the appbar.
* @param {!registry.Console} cons * @param {!registry.Console} cons
* @param {function()} itemTmpl * @param {function()} itemTmpl
* @param {boolean} isEditable
* @constructor * @constructor
* @extends {registry.Component} * @extends {registry.Component}
*/ */
registry.EditItem = function(cons, itemTmpl) { registry.EditItem = function(cons, itemTmpl, isEditable) {
registry.EditItem.base(this, 'constructor', cons); registry.EditItem.base(this, 'constructor', cons);
/** /**
@ -48,6 +49,12 @@ registry.EditItem = function(cons, itemTmpl) {
*/ */
this.id = null; this.id = null;
/**
* Should the "edit" button be enabled?
* @type {boolean}
*/
this.isEditable = isEditable;
/** /**
* Transitional id for next resource during create. * Transitional id for next resource during create.
* @type {?string} * @type {?string}
@ -68,7 +75,7 @@ registry.EditItem.prototype.bindToDom = function(id) {
/** Setup appbar save/edit buttons. */ /** Setup appbar save/edit buttons. */
registry.EditItem.prototype.setupAppbar = function() { registry.EditItem.prototype.setupAppbar = function() {
goog.soy.renderElement(goog.dom.getRequiredElement('reg-appbar'), goog.soy.renderElement(goog.dom.getRequiredElement('reg-app-buttons'),
registry.soy.console.appbarButtons); registry.soy.console.appbarButtons);
goog.events.listen(goog.dom.getRequiredElement('reg-app-btn-add'), goog.events.listen(goog.dom.getRequiredElement('reg-app-btn-add'),
goog.events.EventType.CLICK, goog.events.EventType.CLICK,
@ -85,11 +92,11 @@ registry.EditItem.prototype.setupAppbar = function() {
goog.events.listen(goog.dom.getRequiredElement('reg-app-btn-back'), goog.events.listen(goog.dom.getRequiredElement('reg-app-btn-back'),
goog.events.EventType.CLICK, goog.events.EventType.CLICK,
goog.bind(this.back, this)); goog.bind(this.back, this));
if (this.id) { // Show the add/edit buttons only if isEditable.
registry.util.setVisible('reg-app-btns-edit', true); // "edit" is shown if we have an item's ID
} else { // "add" is shown if we don't have an item's ID
registry.util.setVisible('reg-app-btn-add', true); registry.util.setVisible('reg-app-btns-edit', this.isEditable && !!this.id);
} registry.util.setVisible('reg-app-btn-add', this.isEditable && !this.id);
}; };

View file

@ -39,7 +39,7 @@ goog.forwardDeclare('registry.registrar.Console');
registry.registrar.AdminSettings = function(console, resource) { registry.registrar.AdminSettings = function(console, resource) {
registry.registrar.AdminSettings.base( registry.registrar.AdminSettings.base(
this, 'constructor', console, resource, this, 'constructor', console, resource,
registry.soy.registrar.admin.settings, null); registry.soy.registrar.admin.settings, console.params.isAdmin, null);
}; };
goog.inherits(registry.registrar.AdminSettings, registry.ResourceComponent); goog.inherits(registry.registrar.AdminSettings, registry.ResourceComponent);

View file

@ -45,7 +45,7 @@ goog.forwardDeclare('registry.registrar.Console');
registry.registrar.ContactSettings = function(console, resource) { registry.registrar.ContactSettings = function(console, resource) {
registry.registrar.ContactSettings.base( registry.registrar.ContactSettings.base(
this, 'constructor', console, resource, this, 'constructor', console, resource,
registry.soy.registrar.contacts.contact, null); registry.soy.registrar.contacts.contact, console.params.isOwner, null);
}; };
goog.inherits(registry.registrar.ContactSettings, registry.ResourceComponent); goog.inherits(registry.registrar.ContactSettings, registry.ResourceComponent);

View file

@ -34,7 +34,7 @@ goog.forwardDeclare('registry.registrar.Console');
registry.registrar.ContactUs = function(console, resource) { registry.registrar.ContactUs = function(console, resource) {
registry.registrar.ContactUs.base( registry.registrar.ContactUs.base(
this, 'constructor', console, resource, this, 'constructor', console, resource,
registry.soy.registrar.console.contactUs, null); registry.soy.registrar.console.contactUs, console.params.isOwner, null);
}; };
goog.inherits(registry.registrar.ContactUs, registry.ResourceComponent); goog.inherits(registry.registrar.ContactUs, registry.ResourceComponent);

View file

@ -52,7 +52,7 @@ goog.inherits(registry.registrar.Dashboard, registry.Component);
/** @override */ /** @override */
registry.registrar.Dashboard.prototype.bindToDom = function(id) { registry.registrar.Dashboard.prototype.bindToDom = function(id) {
registry.registrar.Dashboard.base(this, 'bindToDom', ''); registry.registrar.Dashboard.base(this, 'bindToDom', '');
goog.dom.removeChildren(goog.dom.getRequiredElement('reg-appbar')); goog.dom.removeChildren(goog.dom.getRequiredElement('reg-app-buttons'));
goog.soy.renderElement(goog.dom.getElement('reg-content'), goog.soy.renderElement(goog.dom.getElement('reg-content'),
registry.soy.registrar.console.dashboard, registry.soy.registrar.console.dashboard,
this.console.params); this.console.params);

View file

@ -29,6 +29,7 @@ goog.require('registry.registrar.Console');
* @param {string} xsrfToken populated by server-side soy template. * @param {string} xsrfToken populated by server-side soy template.
* @param {string} clientId The registrar clientId. * @param {string} clientId The registrar clientId.
* @param {boolean} isAdmin * @param {boolean} isAdmin
* @param {boolean} isOwner
* @param {string} productName the product name displayed by the UI. * @param {string} productName the product name displayed by the UI.
* @param {string} integrationEmail * @param {string} integrationEmail
* @param {string} supportEmail * @param {string} supportEmail
@ -37,14 +38,15 @@ goog.require('registry.registrar.Console');
* @param {string} technicalDocsUrl * @param {string} technicalDocsUrl
* @export * @export
*/ */
registry.registrar.main = function(xsrfToken, clientId, isAdmin, productName, registry.registrar.main = function(xsrfToken, clientId, isAdmin, isOwner,
integrationEmail, supportEmail, productName, integrationEmail, supportEmail,
announcementsEmail, supportPhoneNumber, announcementsEmail, supportPhoneNumber,
technicalDocsUrl) { technicalDocsUrl) {
new registry.registrar.Console({ new registry.registrar.Console({
xsrfToken: xsrfToken, xsrfToken: xsrfToken,
clientId: clientId, clientId: clientId,
isAdmin: isAdmin, isAdmin: isAdmin,
isOwner: isOwner,
productName: productName, productName: productName,
integrationEmail: integrationEmail, integrationEmail: integrationEmail,
supportEmail: supportEmail, supportEmail: supportEmail,

View file

@ -34,7 +34,7 @@ goog.forwardDeclare('registry.registrar.Console');
registry.registrar.Resources = function(console, resource) { registry.registrar.Resources = function(console, resource) {
registry.registrar.Resources.base( registry.registrar.Resources.base(
this, 'constructor', console, resource, this, 'constructor', console, resource,
registry.soy.registrar.console.resources, null); registry.soy.registrar.console.resources, console.params.isOwner, null);
}; };
goog.inherits(registry.registrar.Resources, goog.inherits(registry.registrar.Resources,
registry.ResourceComponent); registry.ResourceComponent);

View file

@ -39,7 +39,7 @@ goog.forwardDeclare('registry.registrar.Console');
registry.registrar.SecuritySettings = function(console, resource) { registry.registrar.SecuritySettings = function(console, resource) {
registry.registrar.SecuritySettings.base( registry.registrar.SecuritySettings.base(
this, 'constructor', console, resource, this, 'constructor', console, resource,
registry.soy.registrar.security.settings, null); registry.soy.registrar.security.settings, console.params.isOwner, null);
}; };
goog.inherits(registry.registrar.SecuritySettings, registry.ResourceComponent); goog.inherits(registry.registrar.SecuritySettings, registry.ResourceComponent);

View file

@ -34,7 +34,7 @@ goog.forwardDeclare('registry.registrar.Console');
registry.registrar.WhoisSettings = function(console, resource) { registry.registrar.WhoisSettings = function(console, resource) {
registry.registrar.WhoisSettings.base( registry.registrar.WhoisSettings.base(
this, 'constructor', console, resource, this, 'constructor', console, resource,
registry.soy.registrar.whois.settings, null); registry.soy.registrar.whois.settings, console.params.isOwner, null);
}; };
goog.inherits(registry.registrar.WhoisSettings, registry.ResourceComponent); goog.inherits(registry.registrar.WhoisSettings, registry.ResourceComponent);

View file

@ -37,7 +37,7 @@ goog.forwardDeclare('registry.registrar.Console');
registry.registrar.XmlResourceComponent = function( registry.registrar.XmlResourceComponent = function(
itemTmpl, eppTmpls, console) { itemTmpl, eppTmpls, console) {
registry.registrar.XmlResourceComponent.base( registry.registrar.XmlResourceComponent.base(
this, 'constructor', console, itemTmpl); this, 'constructor', console, itemTmpl, console.params.isOwner);
/** @type {!Object} */ /** @type {!Object} */
this.eppTmpls = eppTmpls; this.eppTmpls = eppTmpls;

View file

@ -33,6 +33,7 @@ goog.forwardDeclare('registry.Resource');
* @param {!registry.Console} console console singleton. * @param {!registry.Console} console console singleton.
* @param {!registry.Resource} resource the RESTful resource. * @param {!registry.Resource} resource the RESTful resource.
* @param {!Function} itemTmpl * @param {!Function} itemTmpl
* @param {boolean} isEditable if true, the "edit" button will be enabled
* @param {Function} renderSetCb may be null if this resource is only * @param {Function} renderSetCb may be null if this resource is only
* ever an item. * ever an item.
* @constructor * @constructor
@ -42,8 +43,10 @@ registry.ResourceComponent = function(
console, console,
resource, resource,
itemTmpl, itemTmpl,
isEditable,
renderSetCb) { renderSetCb) {
registry.ResourceComponent.base(this, 'constructor', console, itemTmpl); registry.ResourceComponent.base(
this, 'constructor', console, itemTmpl, isEditable);
/** @type {!registry.Resource} */ /** @type {!registry.Resource} */
this.resource = resource; this.resource = resource;
@ -108,7 +111,7 @@ registry.ResourceComponent.prototype.handleFetchItem = function(id, rsp) {
} else if ('set' in rsp && this.renderSetCb != null) { } else if ('set' in rsp && this.renderSetCb != null) {
// XXX: This conditional logic should be hoisted to edit_item when // XXX: This conditional logic should be hoisted to edit_item when
// collection support is improved. // collection support is improved.
goog.dom.removeChildren(goog.dom.getRequiredElement('reg-appbar')); goog.dom.removeChildren(goog.dom.getRequiredElement('reg-app-buttons'));
this.renderSetCb(goog.dom.getRequiredElement('reg-content'), rsp); this.renderSetCb(goog.dom.getRequiredElement('reg-content'), rsp);
} else { } else {
registry.util.log('unknown message type in handleFetchItem'); registry.util.log('unknown message type in handleFetchItem');

View file

@ -17,6 +17,7 @@ package google.registry.ui.server.registrar;
import static com.google.common.net.HttpHeaders.LOCATION; import static com.google.common.net.HttpHeaders.LOCATION;
import static com.google.common.net.HttpHeaders.X_FRAME_OPTIONS; import static com.google.common.net.HttpHeaders.X_FRAME_OPTIONS;
import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.ADMIN; import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.ADMIN;
import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.OWNER;
import static google.registry.ui.server.registrar.RegistrarConsoleModule.PARAM_CLIENT_ID; import static google.registry.ui.server.registrar.RegistrarConsoleModule.PARAM_CLIENT_ID;
import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY; import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY;
@ -141,6 +142,7 @@ public final class ConsoleUiAction implements Runnable {
try { try {
clientId = paramClientId.orElse(registrarAccessor.guessClientId()); clientId = paramClientId.orElse(registrarAccessor.guessClientId());
data.put("clientId", clientId); data.put("clientId", clientId);
data.put("isOwner", roleMap.containsEntry(clientId, OWNER));
data.put("isAdmin", roleMap.containsEntry(clientId, ADMIN)); data.put("isAdmin", roleMap.containsEntry(clientId, ADMIN));
// We want to load the registrar even if we won't use it later (even if we remove the // We want to load the registrar even if we won't use it later (even if we remove the

View file

@ -102,7 +102,6 @@
/** Appbar add/back, edit/cancel appbar. */ /** Appbar add/back, edit/cancel appbar. */
{template .appbarButtons} {template .appbarButtons}
<div id="reg-app-buttons" class="{css('kd-buttonbar')} {css('left')}">
<button id="reg-app-btn-add" <button id="reg-app-btn-add"
type="button" type="button"
class="{css('kd-button')} {css('kd-button-submit')} {css('hidden')}"> class="{css('kd-button')} {css('kd-button-submit')} {css('hidden')}">
@ -123,7 +122,6 @@
<button id="reg-app-btn-cancel" type="button" class="{css('kd-button')}"> <button id="reg-app-btn-cancel" type="button" class="{css('kd-button')}">
Cancel</button> Cancel</button>
</div> </div>
</div>
{/template} {/template}

View file

@ -18,14 +18,9 @@
/** Registrar admin settings page for view and edit. */ /** Registrar admin settings page for view and edit. */
{template .settings} {template .settings}
{@param clientId: string}
{@param allowedTlds: list<string>} {@param allowedTlds: list<string>}
{@param readonly: bool}
<form name="item" class="{css('item')} {css('registrar')}"> <form name="item" class="{css('item')} {css('registrar')}">
<h1>Administrator settings for {$clientId}</h1> <h1>Administrator settings</h1>
{if $readonly}
<p>Use the 'Edit' button above to switch to enable editing the information below.
{/if}
<table> <table>
<tr class="{css('kd-settings-pane-section')}"> <tr class="{css('kd-settings-pane-section')}">
<td> <td>

View file

@ -25,6 +25,7 @@
{@param clientId: string} /** Registrar client identifier. */ {@param clientId: string} /** Registrar client identifier. */
{@param allClientIds: list<string>} /** All registrar client identifiers for the user. */ {@param allClientIds: list<string>} /** All registrar client identifiers for the user. */
{@param isAdmin: bool} {@param isAdmin: bool}
{@param isOwner: bool}
{@param username: string} /** Arbitrary username to display. */ {@param username: string} /** Arbitrary username to display. */
{@param logoutUrl: string} /** Generated URL for logging out of Google. */ {@param logoutUrl: string} /** Generated URL for logging out of Google. */
{@param productName: string} /** Name to display for this software product. */ {@param productName: string} /** Name to display for this software product. */
@ -40,7 +41,17 @@
{/call} {/call}
{call registry.soy.console.googlebar data="all" /} {call registry.soy.console.googlebar data="all" /}
<div id="reg-app"> <div id="reg-app">
<div id="reg-appbar" class="{css('kd-appbar')}"></div> <div id="reg-appbar" class="{css('kd-appbar')}">
<div class="{css('kd-description')}">
Accessing <span class="{css('kd-value')}">{$clientId}</span> as{sp}
{if $isOwner}<span class="{css('kd-value')}">Owner</span>{/if}
{if $isAdmin}<span class="{css('kd-value')}">Admin</span>{/if}
{if length($allClientIds) > 1}
{sp}(Switch registrar: {call .clientIdSelect_ data="all" /})
{/if}
</div>
<div id="reg-app-buttons" class="{css('kd-buttonbar')} {css('left')}"></div>
</div>
{call .navbar_ data="all" /} {call .navbar_ data="all" /}
<div id="reg-content-and-footer"> <div id="reg-content-and-footer">
<div id="reg-content"> <div id="reg-content">
@ -65,6 +76,7 @@
registry.registrar.main({$xsrfToken}, registry.registrar.main({$xsrfToken},
{$clientId}, {$clientId},
{if $isAdmin}true{else}false{/if}, {if $isAdmin}true{else}false{/if},
{if $isOwner}true{else}false{/if},
{$productName}, {$productName},
{$integrationEmail}, {$integrationEmail},
{$supportEmail}, {$supportEmail},
@ -78,22 +90,9 @@
/** Sidebar nav. Ids on each elt for testing only. */ /** Sidebar nav. Ids on each elt for testing only. */
{template .navbar_ visibility="private"} {template .navbar_ visibility="private"}
{@param clientId: string} /** Registrar client identifier. */
{@param allClientIds: list<string>}
{@param isAdmin: bool} {@param isAdmin: bool}
<div id="reg-nav" class="{css('kd-content-sidebar')}"> <div id="reg-nav" class="{css('kd-content-sidebar')}">
<form>
<select name="clientId"
id="select-client-id"
class="{css('kd-button')} {css('kd-button-submit')}"
onchange='this.form.submit()'>
<option value="">[auto select]</option>
{for $id in $allClientIds}
<option value="{$id}" {if $id == $clientId}selected{/if}>{$id}</option>
{/for}
</select>
</form>
<ul id="reg-navlist"> <ul id="reg-navlist">
<li> <li>
<a href="#">Home</a> <a href="#">Home</a>
@ -120,6 +119,24 @@
{/template} {/template}
/** Drop-down selection for the clientId. */
{template .clientIdSelect_}
{@param clientId: string} /** Registrar client identifier. */
{@param allClientIds: list<string>}
<form>
<select name="clientId"
id="select-client-id"
class="{css('kd-button')} {css('kd-button-submit')}"
onchange='this.form.submit()'>
<option value="">[auto select]</option>
{for $id in $allClientIds}
<option value="{$id}" {if $id == $clientId}selected{/if}>{$id}</option>
{/for}
</select>
</form>
{/template}
/** /**
* Feature disabled * Feature disabled
*/ */

View file

@ -25,9 +25,6 @@
{@param readonly: bool} {@param readonly: bool}
<form name="item" class="{css('item')} {css('registrar')}"> <form name="item" class="{css('item')} {css('registrar')}">
<h1>Security settings</h1> <h1>Security settings</h1>
{if $readonly}
<p>Use the 'Edit' button above to switch to enable editing the information below.
{/if}
<table> <table>
<tr class="{css('kd-settings-pane-section')}"> <tr class="{css('kd-settings-pane-section')}">
<td> <td>

View file

@ -61,6 +61,7 @@ registry.registrar.ConsoleTestUtil.renderConsoleMain = function(
username: args.username || 'jart', username: args.username || 'jart',
logoutUrl: args.logoutUrl || 'https://logout.url.com', logoutUrl: args.logoutUrl || 'https://logout.url.com',
isAdmin: !!args.isAdmin, isAdmin: !!args.isAdmin,
isOwner: !!args.isOwner,
clientId: args.clientId || 'ignore', clientId: args.clientId || 'ignore',
allClientIds: args.allClientIds || ['clientId1', 'clientId2'], allClientIds: args.allClientIds || ['clientId1', 'clientId2'],
logoFilename: args.logoFilename || 'logo.png', logoFilename: args.logoFilename || 'logo.png',
@ -89,6 +90,15 @@ registry.registrar.ConsoleTestUtil.visit = function(
opt_args.clientId = opt_args.clientId || 'dummyRegistrarId'; opt_args.clientId = opt_args.clientId || 'dummyRegistrarId';
opt_args.xsrfToken = opt_args.xsrfToken || 'dummyXsrfToken'; opt_args.xsrfToken = opt_args.xsrfToken || 'dummyXsrfToken';
opt_args.isAdmin = !!opt_args.isAdmin; opt_args.isAdmin = !!opt_args.isAdmin;
// set the default isOwner to be the opposite of isAdmin.
// That way, if we don't explicitly state them both we get what we'd expect:
// {} -> OWNER (the "regular" case of a visitor to the console)
// {isOwner:true} -> OWNER
// {isAdmin:true} -> ADMIN (the "regular" case of an admin visitor)
// {isOwner:true, isAdmin:true} -> OWNER + ADMIN together
if (opt_args.isOwner === undefined) {
opt_args.isOwner = !opt_args.isAdmin;
}
if (opt_args.isEppLoggedIn === undefined) { if (opt_args.isEppLoggedIn === undefined) {
opt_args.isEppLoggedIn = true; opt_args.isEppLoggedIn = true;
} }