[ZEPPELIN-1054] Improve "Credentials" UI #1100

AhyoungRyu · 2016-06-28T08:04:29Z

What is this PR for?

Currently, users can add new their credential info for data source authentication in Zeppelin "Credentials" menu. Even though it was saved successfully, they can't see the whole list of credentials in Zeppelin UI.
This PR enables to get all credential list, edit and remove via UI.

NOTE : Since this patch was implemented based on #1030 API, should be tested after #1030 merged.

What type of PR is it?

Improvement & Documentation

Todos

- rename interpreter_authorization.md -> datasource_authorization.md
- remove Interpreter Authorization section (since we don't have this feature yet : ZEPPELIN-945)
- rebase after [ZEPPELIN-1023] Add more credential apis. #1030 & BugFix-blocking of blank values insertion on the Credential page. #1064 merged
- address reviews

What is the Jira issue?

ZEPPELIN-1054

How should this be tested?

Apply this patch and build zeppelin-web as described in here.
Go to Credentials menu.
Add new credentials -> you can see the credential info in the credential list table.
You can edit & delete them. -> Compare with conf/credentials.json

Screenshots (if appropriate)

Before
After

If there is no credential
datasource_authorization.md

Questions:

Does the licenses files need update? no
Is there breaking changes for older versions? no
Does this needs documentation? yes

bzz · 2016-06-28T10:18:38Z

Looks good to me, \cc @corneadoug @felizbear for review of the frontend part

astroshim · 2016-06-28T13:27:18Z

Really great feature! Thanks for making this.

heruka-urgyen · 2016-06-28T15:33:13Z

zeppelin-web/src/app/credential/credential.controller.js

+    $scope.credentialPassword = '';
+  };
+
+  $scope.copyOriginCredentialsInfo = function(entity, username, password) {


It looks to me that this function doesn't do anything at all. it copies input arguments but doesn't store these copies anywhere or return anything

heruka-urgyen · 2016-06-28T15:43:37Z

Not sure how to test in the browser because I have to log in, but I have no clue how to register. And don't really understand why I have to do it to access settings.

Link to documentation on the main page is broken.

AhyoungRyu · 2016-06-28T15:57:31Z

@felizbear This PR can be tested after #1030 merged as I described in the above.

Register : you don't need to register if you don't want. It needs shiro authentication setting. But still you can add your credential info as anonymous, even if you didn't activate shiro. It's a default setting. I think this information need to be added to docs too. Users might be confused too.
Docs link : Regarding docs link, you can't see currently since this docs was newly added by this PR. If you want to see this docs, you have to build the website in your local as described in here. It's little bit tiresome, so I added screenshot images.

And appreciate for your feedbacks. I'll take a look at it :)

Leemoonsoo · 2016-07-04T23:43:58Z

I'm keep getting GET http://localhost:8080/api/credential 405 (Method Not Allowed), when i visit credential menu. @AhyoungRyu Could you check?

AhyoungRyu · 2016-07-05T07:25:04Z

@Leemoonsoo Since this PR needs #1030's extended API, I rebased from master. Maybe now it works properly.

AhyoungRyu · 2016-07-05T07:25:19Z

Ready for review.

bzz · 2016-07-05T08:08:58Z

CI failure is not related and is fixed under ZEPPELIN-1063

Leemoonsoo · 2016-07-05T15:11:31Z

Tested and overall working well. Found two (minor) problems

When i login from Credential page, list not refreshed after logged in

"Edit" -> "Save" -> "Cancel" -> "Cancel" does not update actual credential in the backend. However, it displays modified value in frontend, until page is refreshed.

corneadoug · 2016-07-06T05:33:18Z

In the case of login from credential page, its a separate issue, you shouldn't be able to do that.
I created an issue: https://issues.apache.org/jira/browse/ZEPPELIN-1123 and will take care of it.

corneadoug · 2016-07-06T06:36:02Z

zeppelin-web/src/app/credential/credential.controller.js

+  };
+
+  var init = function() {
+    $scope.resetCredentialInfo();


This isnt needed here

corneadoug · 2016-07-06T06:43:43Z

Latest commit (41413df) can also be reverted

corneadoug · 2016-07-06T08:07:53Z

zeppelin-web/src/app/credential/credential.controller.js

+  $scope.addNewCredentialInfo = function() {
+    if ($scope.entity && _.isEmpty($scope.entity.trim()) &&
+      $scope.username && _.isEmpty($scope.username.trim())) {
      BootstrapDialog.alert({


Let's replace this by ngtoast

corneadoug · 2016-07-06T08:11:25Z

zeppelin-web/src/app/credential/credential.controller.js

+          success(function(data, status, headers, config) {
+            var index = _.findIndex($scope.credentialInfo, { 'entity': entity });
+            $scope.credentialInfo.splice(index, 1);
+            //getCredentialInfo();


Sorry I left some things commented :( Needs to be removed

AhyoungRyu · 2016-07-06T08:38:29Z

@corneadoug Really appreciate for your effort! I addressed your last comments.

corneadoug · 2016-07-06T08:45:47Z

zeppelin-web/src/app/credential/credential.controller.js

-        'username': $scope.credentialUsername,
-        'password': $scope.credentialPassword
-      } ).
+    var data  = {


We need to change the name of the variable here, let's say newCredential

AhyoungRyu · 2016-07-07T04:49:24Z

@corneadoug All CI builds failed. Seems they are related with ZEPPELIN-1016 or ZEPPELIN-1126.

corneadoug · 2016-07-08T02:41:00Z

@AhyoungRyu Can you rebase your branch please?

AhyoungRyu · 2016-07-08T03:32:22Z

@corneadoug yeah I did :)

corneadoug · 2016-07-08T04:01:04Z

CI is green, merging if there is no more discussion

@prabhjyotsingh

… and configurations info ### What is this PR for? For some user cases, people might want to hide **Interpreter Setting**, **Credentials** and **Configurations** information to other users (who are defined in `conf/shiro.ini`). So I added ``` #/api/interpreter/** = authc, roles[admin] #/api/configurations/** = authc, roles[admin] #/api/credential/** = authc, roles[admin] ``` below the [ [urls] ](https://github.com/apache/zeppelin/blob/master/conf/shiro.ini#L38) section. This issue was originally suggested at [Zeppelin user mailing list](https://mail-archives.apache.org/mod_mbox/zeppelin-users/201606.mbox/%3CCAPgU7Y%3DBJrXQ_P0ond4PTukoya0FEjwoPuUb31iN3qwo8iyM1Q%40mail.gmail.com%3E) by TomNorden ### What type of PR is it? Improvement | Documentation ### Todos * [x] - Add `interpreter`, `credential` and `configuration` url to `conf/shiro.ini` * [x] - Update `shiroauthentication.md` for this change * [x] - Redirect to home with ngToast error message when status is `401` * [x] - Rebase after #1100 merged and add error message to `Credential` menu as well ### What is the Jira issue? [ZEPPELIN-987](https://issues.apache.org/jira/browse/ZEPPELIN-987) ### How should this be tested? 1. Apply this patch and restart Zeppelin 2. Login with `admin` and `password1` 3. Go to interpreter, credential and configuration tab -> You can see all of the information in each tabs 4. Logout -> Login again with `user1` and `password2` 5. Go to interpreter, credential and configuration tab -> In this time, you can't see all of the information in each tabs ### Screenshots (if appropriate) - When you login with `user1` (doesn't have permission to see the interpreter, credential and cofiguration info) - interpreter menu ![interpreters](https://cloud.githubusercontent.com/assets/10060731/16708520/bedc8732-4631-11e6-938c-ff41d1fbab93.gif) - configuration menu ![configurations](https://cloud.githubusercontent.com/assets/10060731/16708525/ce5eb7c0-4631-11e6-9f36-8b97e2b7914a.gif) - credential menu ![credential-after](https://cloud.githubusercontent.com/assets/10060731/16726180/e56cfa52-4795-11e6-9a5d-740681092e96.gif) - `shiroauthentication.md` <img width="807" alt="screen shot 2016-06-10 at 12 25 02 pm" src="https://cloud.githubusercontent.com/assets/10060731/15976949/a49bc542-2f0a-11e6-8869-8575ba8f1875.png"> ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? Yes, so I updated. Author: AhyoungRyu <fbdkdud93@hanmail.net> Closes #993 from AhyoungRyu/ZEPPELIN-987 and squashes the following commits: 1d291ac [AhyoungRyu] Redirect to home when unauthorized user click 'credentials' 5896c12 [AhyoungRyu] Revert shiro setting 4411188 [AhyoungRyu] Address @prabhjyotsingh feedback 5c9242c [AhyoungRyu] Redirect to home with error message when status is 401 2a054d4 [AhyoungRyu] Add interpreter, credential and configuration urls to shiro.ini d3a81d5 [AhyoungRyu] Update shiro authentication docs 8be7970 [AhyoungRyu] Change authcBasic -> authc

@Leemoonsoo

### What is this PR for? Currently, users can add new their credential info for data source authentication in Zeppelin "Credentials" menu. Even though it was saved successfully, they can't see the whole list of credentials in Zeppelin UI. This PR enables to `get` all credential list, `edit` and `remove` via UI. *NOTE : Since this patch was implemented based on apache#1030 API, should be tested after apache#1030 merged.* ### What type of PR is it? Improvement & Documentation ### Todos * [x] - rename `interpreter_authorization.md` -> `datasource_authorization.md` * [x] - remove `Interpreter Authorization` section (since we don't have this feature yet : [ZEPPELIN-945](https://issues.apache.org/jira/browse/ZEPPELIN-945)) * [x] - rebase after apache#1030 & apache#1064 merged * [ ] - address reviews ### What is the Jira issue? [ZEPPELIN-1054](https://issues.apache.org/jira/browse/ZEPPELIN-1054) ### How should this be tested? 1. Apply this patch and build `zeppelin-web` as described in [here](https://github.com/apache/zeppelin/tree/master/zeppelin-web#configured-environment). 2. Go to `Credentials` menu. 3. Add new credentials -> you can see the credential info in the credential list table. 4. You can edit & delete them. -> Compare with `conf/credentials.json` ### Screenshots (if appropriate) - Before <img width="952" alt="screen shot 2016-06-28 at 12 37 10 am" src="https://cloud.githubusercontent.com/assets/10060731/16407604/69b0c4d8-3cc9-11e6-8284-9abe2969cdc1.png"> - After ![add_credential](https://cloud.githubusercontent.com/assets/10060731/16576765/3671aa16-42cc-11e6-9d9f-dfe1f33f8d37.gif) If there is no credential <img width="957" alt="screen shot 2016-06-28 at 12 19 46 am" src="https://cloud.githubusercontent.com/assets/10060731/16407620/7838995e-3cc9-11e6-90ba-1bd0173a1b49.png"> - `datasource_authorization.md` <img width="845" alt="screen shot 2016-06-28 at 7 58 24 pm" src="https://cloud.githubusercontent.com/assets/10060731/16439169/d4026034-3d6a-11e6-930f-86de12e5fc49.png"> <img width="851" alt="screen shot 2016-06-28 at 7 58 44 pm" src="https://cloud.githubusercontent.com/assets/10060731/16439170/d62f2842-3d6a-11e6-9d3f-ecc5cda29c77.png"> <img width="846" alt="screen shot 2016-06-28 at 8 00 20 pm" src="https://cloud.githubusercontent.com/assets/10060731/16439200/fed58390-3d6a-11e6-9aa2-8cff5a1b7b66.png"> ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? yes Author: AhyoungRyu <fbdkdud93@hanmail.net> Closes apache#1100 from AhyoungRyu/ZEPPELIN-1054 and squashes the following commits: 7c38c90 [AhyoungRyu] Fix checkstyle error with jscs rule ab9814c [AhyoungRyu] Remove cancelCredentialInfoUpdate() 899bb15 [AhyoungRyu] Fix a bug reported by @Leemoonsoo 57cb280 [AhyoungRyu] Make focusing to text inputbox after update cancel cea8c93 [AhyoungRyu] Fix typos in datasource_authorization.md cc72ae8 [AhyoungRyu] update xeditable license version c100a64 [AhyoungRyu] Delete interpreter_authorization.md 304e684 [AhyoungRyu] Add datasource_authorization.md docs 5768604 [AhyoungRyu] Add datasource_authorization.md to index & navi menu 64bf6fe [AhyoungRyu] Update angular-xeditable version 573c3d1 [AhyoungRyu] Enable credential info to get list, edit and remove via UI

@prabhjyotsingh

… and configurations info ### What is this PR for? For some user cases, people might want to hide **Interpreter Setting**, **Credentials** and **Configurations** information to other users (who are defined in `conf/shiro.ini`). So I added ``` #/api/interpreter/** = authc, roles[admin] #/api/configurations/** = authc, roles[admin] #/api/credential/** = authc, roles[admin] ``` below the [ [urls] ](https://github.com/apache/zeppelin/blob/master/conf/shiro.ini#L38) section. This issue was originally suggested at [Zeppelin user mailing list](https://mail-archives.apache.org/mod_mbox/zeppelin-users/201606.mbox/%3CCAPgU7Y%3DBJrXQ_P0ond4PTukoya0FEjwoPuUb31iN3qwo8iyM1Q%40mail.gmail.com%3E) by TomNorden ### What type of PR is it? Improvement | Documentation ### Todos * [x] - Add `interpreter`, `credential` and `configuration` url to `conf/shiro.ini` * [x] - Update `shiroauthentication.md` for this change * [x] - Redirect to home with ngToast error message when status is `401` * [x] - Rebase after apache#1100 merged and add error message to `Credential` menu as well ### What is the Jira issue? [ZEPPELIN-987](https://issues.apache.org/jira/browse/ZEPPELIN-987) ### How should this be tested? 1. Apply this patch and restart Zeppelin 2. Login with `admin` and `password1` 3. Go to interpreter, credential and configuration tab -> You can see all of the information in each tabs 4. Logout -> Login again with `user1` and `password2` 5. Go to interpreter, credential and configuration tab -> In this time, you can't see all of the information in each tabs ### Screenshots (if appropriate) - When you login with `user1` (doesn't have permission to see the interpreter, credential and cofiguration info) - interpreter menu ![interpreters](https://cloud.githubusercontent.com/assets/10060731/16708520/bedc8732-4631-11e6-938c-ff41d1fbab93.gif) - configuration menu ![configurations](https://cloud.githubusercontent.com/assets/10060731/16708525/ce5eb7c0-4631-11e6-9f36-8b97e2b7914a.gif) - credential menu ![credential-after](https://cloud.githubusercontent.com/assets/10060731/16726180/e56cfa52-4795-11e6-9a5d-740681092e96.gif) - `shiroauthentication.md` <img width="807" alt="screen shot 2016-06-10 at 12 25 02 pm" src="https://cloud.githubusercontent.com/assets/10060731/15976949/a49bc542-2f0a-11e6-8869-8575ba8f1875.png"> ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? Yes, so I updated. Author: AhyoungRyu <fbdkdud93@hanmail.net> Closes apache#993 from AhyoungRyu/ZEPPELIN-987 and squashes the following commits: 1d291ac [AhyoungRyu] Redirect to home when unauthorized user click 'credentials' 5896c12 [AhyoungRyu] Revert shiro setting 4411188 [AhyoungRyu] Address @prabhjyotsingh feedback 5c9242c [AhyoungRyu] Redirect to home with error message when status is 401 2a054d4 [AhyoungRyu] Add interpreter, credential and configuration urls to shiro.ini d3a81d5 [AhyoungRyu] Update shiro authentication docs 8be7970 [AhyoungRyu] Change authcBasic -> authc

heruka-urgyen reviewed Jun 28, 2016
View reviewed changes

AhyoungRyu force-pushed the ZEPPELIN-1054 branch from 9e341d6 to 3b32ec4 Compare July 5, 2016 03:07

bzz mentioned this pull request Jul 5, 2016

ZEPPELIN-1063: fix PythonInterpreter flaky test #1134

Closed

corneadoug reviewed Jul 6, 2016
View reviewed changes

AhyoungRyu force-pushed the ZEPPELIN-1054 branch from 41413df to c3023bd Compare July 6, 2016 07:06

corneadoug reviewed Jul 6, 2016
View reviewed changes

AhyoungRyu force-pushed the ZEPPELIN-1054 branch from cd0b325 to 055fe94 Compare July 8, 2016 03:30

AhyoungRyu added 10 commits July 9, 2016 22:07

Enable credential info to get list, edit and remove via UI

573c3d1

Update angular-xeditable version

64bf6fe

Add datasource_authorization.md to index & navi menu

5768604

Add datasource_authorization.md docs

304e684

Delete interpreter_authorization.md

c100a64

update xeditable license version

cc72ae8

Fix typos in datasource_authorization.md

cea8c93

Make focusing to text inputbox after update cancel

57cb280

Fix a bug reported by @Leemoonsoo

899bb15

Remove cancelCredentialInfoUpdate()

ab9814c

AhyoungRyu force-pushed the ZEPPELIN-1054 branch from 055fe94 to 7c38c90 Compare July 9, 2016 13:47

Fix checkstyle error with jscs rule

7c38c90

AhyoungRyu mentioned this pull request Jul 9, 2016

[ZEPPELIN-987] Enable user to secure interpreter setting, credentials and configurations info #993

Closed

4 tasks

asfgit closed this in d87f2e5 Jul 11, 2016

[ZEPPELIN-1054] Improve "Credentials" UI #1100

[ZEPPELIN-1054] Improve "Credentials" UI #1100

Uh oh!

Conversation

AhyoungRyu commented Jun 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is this PR for?

What type of PR is it?

Todos

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

Uh oh!

bzz commented Jun 28, 2016

Uh oh!

astroshim commented Jun 28, 2016

Uh oh!

heruka-urgyen Jun 28, 2016

Choose a reason for hiding this comment

Uh oh!

heruka-urgyen commented Jun 28, 2016

Uh oh!

AhyoungRyu commented Jun 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Leemoonsoo commented Jul 4, 2016

Uh oh!

AhyoungRyu commented Jul 5, 2016

Uh oh!

AhyoungRyu commented Jul 5, 2016

Uh oh!

bzz commented Jul 5, 2016

Uh oh!

Leemoonsoo commented Jul 5, 2016

Uh oh!

corneadoug commented Jul 6, 2016

Uh oh!

corneadoug Jul 6, 2016

Choose a reason for hiding this comment

Uh oh!

corneadoug commented Jul 6, 2016

Uh oh!

corneadoug Jul 6, 2016

Choose a reason for hiding this comment

Uh oh!

corneadoug Jul 6, 2016

Choose a reason for hiding this comment

Uh oh!

AhyoungRyu commented Jul 6, 2016

Uh oh!

corneadoug Jul 6, 2016

Choose a reason for hiding this comment

Uh oh!

AhyoungRyu commented Jul 7, 2016

Uh oh!

corneadoug commented Jul 8, 2016

Uh oh!

AhyoungRyu commented Jul 8, 2016

Uh oh!

corneadoug commented Jul 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

AhyoungRyu commented Jun 28, 2016 •

edited

Loading

AhyoungRyu commented Jun 28, 2016 •

edited

Loading