Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1152#discussion_r62336714
--- Diff:
api/src/org/apache/cloudstack/api/command/admin/user/GetUserKeysCmd.java ---
@@ -0,0 +1,74 @@
+// Licensed to the Apache Software Foundati
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1152#discussion_r62337172
--- Diff: server/test/com/cloud/user/MockAccountManagerImpl.java ---
@@ -401,5 +403,24 @@ public Long finalyzeAccountId(String accountName, Long
domainI
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1152#discussion_r62337077
--- Diff: server/test/com/cloud/user/MockAccountManagerImpl.java ---
@@ -401,5 +403,24 @@ public Long finalyzeAccountId(String accountName, Long
domainI
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1152#discussion_r62337130
--- Diff: server/test/com/cloud/user/MockAccountManagerImpl.java ---
@@ -401,5 +403,24 @@ public Long finalyzeAccountId(String accountName, Long
domainI
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1152#discussion_r62336941
--- Diff: server/src/com/cloud/user/AccountManager.java ---
@@ -198,4 +200,11 @@ void buildACLViewSearchCriteria(SearchCriteria s
public static
Github user kansal commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-216217062
@DaanHoogland sure. Will rebase and keep the default value to false.
---
If your project is set up for it, you can reply to this email and have your
reply appear o
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-216215437
@kansal please go ahead and remove the key from the response. We'll test
run it and add fixes to tests if needed. (cc @rhtyd my last comment is still
valid)
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-216206238
@kansal can you rebase against latest master and share state of your PR,
thanks
@DaanHoogland @jburwell do we still have outstanding issues on PR; do we
want
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-202852425
@kansal as the complexity is unknown I would just go ahead and update the
pr. We'll see the damage and think of fixes as we go. As for setting the value
to tru
Github user kansal commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-202844861
@DaanHoogland I am not sure of the amount of work that needs to be done for
fixing all the existing test cases. Will revisit this and update. I still
personally thin
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-202822605
@kansal did you get to this yet?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your projec
Github user kansal commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1152#discussion_r48704472
--- Diff: server/src/com/cloud/user/AccountManager.java ---
@@ -198,4 +200,11 @@ void buildACLViewSearchCriteria(SearchCriteria s
public static fi
Github user kansal commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-168581991
cc @DaanHoogland @jburwell Okay. Agreed with that. So I am setting the
default value to false but for running tests and maybe some other existing
integration we wil
Github user jburwell commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-168570237
@DaanHoogland I complete agree with you regarding exposing credential
information. The best practice when credentials are lost is to require that
they be changed.
Github user remibergsma commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1152#discussion_r48695937
--- Diff: server/src/com/cloud/user/AccountManager.java ---
@@ -198,4 +200,11 @@ void buildACLViewSearchCriteria(SearchCriteria s
public stat
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-168527559
@kansal I don't agree that making noise first is the way to go. We should
disable the return of the key first and document it. Security demands that we
play it
Github user kansal commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-168295517
@DaanHoogland Agreed with that point. But its not only about the testing.
I'm sure many people will be using it in their own integration. I think we
should not chang
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-168200764
@kansal when you say 'I have deprecated that as many regressions were using
the secret key from those APIs for authentication', I think we should adjust
those
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1152#discussion_r48657695
--- Diff: server/src/com/cloud/user/AccountManager.java ---
@@ -198,4 +200,11 @@ void buildACLViewSearchCriteria(SearchCriteria s
public sta
Github user kansal commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-165426036
Have updated this PR. Instead of directly removing the secret key from
response, I have deprecated that as many regressions were using the secret key
from those APIs
Github user kansal commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-163539799
@DaanHoogland Sure will try. Will take some time as I have to go through
the documentation first.
---
If your project is set up for it, you can reply to this email
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-162869020
@kansal looks good, but for a change like this I would like a marvin test
to prove it and guarantee it's continued functioning/functionality Do you see
chance
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1152#discussion_r46947731
--- Diff:
api/src/org/apache/cloudstack/api/command/admin/user/ListKeysCmd.java ---
@@ -0,0 +1,74 @@
+// Licensed to the Apache Software Foundat
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1152#discussion_r46947711
--- Diff:
api/src/org/apache/cloudstack/api/command/admin/user/ListKeysCmd.java ---
@@ -0,0 +1,74 @@
+// Licensed to the Apache Software Foundat
Github user kansal commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-162867278
@jburwell @DaanHoogland @kishankavala Have included some changes related
to the UI. Now after generating the keys from UI, after ListUserCmd() api,
listKeysCmd() wi
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-162342597
@kansal looking forward to your update. your intended change makes sense
---
If your project is set up for it, you can reply to this email and have your
reply
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1152#discussion_r46775072
--- Diff: api/src/com/cloud/user/AccountService.java ---
@@ -136,4 +140,6 @@ void checkAccess(Account account, AccessType
accessType, boolean sameOw
Github user kansal commented on the pull request:
https://github.com/apache/cloudstack/pull/1152#issuecomment-161944643
@jburwell Sure!! Will look into these. Adding the test cases and some UI
changes for this to work. Will update the PR.
---
If your project is set up for it, you can
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1152#discussion_r46649508
--- Diff: api/src/com/cloud/user/AccountService.java ---
@@ -136,4 +140,6 @@ void checkAccess(Account account, AccessType
accessType, boolean sameOwner,
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1152#discussion_r46649429
--- Diff:
api/src/org/apache/cloudstack/api/command/admin/user/ListKeysCmd.java ---
@@ -0,0 +1,72 @@
+// Licensed to the Apache Software Foundation
GitHub user kansal opened a pull request:
https://github.com/apache/cloudstack/pull/1152
CLOUDSTACK-9099: SecretKey is returned from the APIs - Fixed
The current implementation of User and account management API (in general)
return the secret key as a user or account response.
31 matches
Mail list logo
