Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(734)

Issue 7301105: Go environment authentication:

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 1 month ago by frankban
Modified:
11 years, 1 month ago
Reviewers:
bcsaller, mp+149360
Visibility:
Public.

Description

Go environment authentication: - connecting to juju-core; - authenticating; - unit tests for the authorization process. Note that currently juju-core accepts any password; this will change soon. QA steps: - bootstrap using juju-core; - get the boostrap node hostname from "juju status"; - accept the SSL certificate at https://hostname:17070/; this currently works with Firefox, but not with Chromium; - in the Juju GUI config (either app/config-debug.js or app/config-prod.js, see below), set "socket_url" to wss://hostname:17070/ (same hostname as above); - in the Juju GUI config, set "apiBackend" to "go". Run "make devel" (if you changed app/config-debug.js) or "make prod" (if you changed app/config-prod.js), and check that you can login to the Juju GUI. https://code.launchpad.net/~frankban/juju-gui/bug-1123660-go-env-authentication/+merge/149360 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 4

Patch Set 2 : Go environment authentication: #

Patch Set 3 : Go environment authentication: #

Unified diffs Side-by-side diffs Delta from patch set Stats (+208 lines, -21 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M app/app.js View 1 chunk +6 lines, -1 line 0 comments Download
M app/index.html View 1 chunk +1 line, -1 line 0 comments Download
M app/store/env/base.js View 2 chunks +4 lines, -2 lines 0 comments Download
M app/store/env/go.js View 1 3 chunks +93 lines, -3 lines 0 comments Download
M app/store/env/python.js View 3 chunks +9 lines, -2 lines 0 comments Download
M app/templates/login.handlebars View 1 chunk +1 line, -1 line 0 comments Download
M app/views/login.js View 1 chunk +1 line, -0 lines 0 comments Download
M test/index.html View 1 chunk +1 line, -0 lines 0 comments Download
M test/test_app.js View 6 chunks +19 lines, -10 lines 0 comments Download
A test/test_env_go.js View 1 chunk +71 lines, -0 lines 0 comments Download
M undocumented View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 3
frankban
Please take a look.
11 years, 1 month ago (2013-02-19 18:30:30 UTC) #1
bcsaller
LGTM, thanks :) https://codereview.appspot.com/7301105/diff/1/app/app.js File app/app.js (right): https://codereview.appspot.com/7301105/diff/1/app/app.js#newcode307 app/app.js:307: this.env.login(); This check is already in ...
11 years, 1 month ago (2013-02-19 22:28:16 UTC) #2
frankban
11 years, 1 month ago (2013-02-20 12:18:47 UTC) #3
*** Submitted:

Go environment authentication:

- connecting to juju-core;
- authenticating;
- unit tests for the authorization process.

Note that currently juju-core accepts any password; this will change soon.

QA steps:

- bootstrap using juju-core;
- get the boostrap node hostname from "juju status";
- accept the SSL certificate at https://hostname:17070/;
  this currently works with Firefox, but not with Chromium;
- in the Juju GUI config (either app/config-debug.js or app/config-prod.js,
  see below), set "socket_url" to wss://hostname:17070/ (same hostname as
  above);
- in the Juju GUI config, set "apiBackend" to "go".

Run "make devel" (if you changed app/config-debug.js) or "make prod" (if you
changed app/config-prod.js), and check that you can login to the Juju GUI.

R=bcsaller
CC=
https://codereview.appspot.com/7301105

https://codereview.appspot.com/7301105/diff/1/app/app.js
File app/app.js (right):

https://codereview.appspot.com/7301105/diff/1/app/app.js#newcode307
app/app.js:307: this.env.login();
On 2013/02/19 22:28:16, bcsaller wrote:
> This check is already in env.login(), do we need it here?

Yes it is already in the underlying env layer, but it has a different semantic
here. There it is just a check for the API to be called correctly (otherwise an
alert is emitted); here, we just use the API as it is designed: only call
login() if user and password are set in the env object.
Moreover, without this check, we would always see the alert from within the env
login, if user and password are not passed from the config file.

https://codereview.appspot.com/7301105/diff/1/app/store/env/go.js
File app/store/env/go.js (right):

https://codereview.appspot.com/7301105/diff/1/app/store/env/go.js#newcode50
app/store/env/go.js:50: if ('RequestId' in data) {
On 2013/02/19 22:28:16, bcsaller wrote:
> Don't all API calls now use request ids and handles from rpc will all have an
> id? Not clean when it wouldn't be in data.

You are right, we were maybe too much defensive here. Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b