----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15789/#review29314 -----------------------------------------------------------
Generally, try to add test cases where every there is significant logic. https://svn.apache.org/repos/asf/airavata/trunk/modules/credential-store/src/main/java/org/apache/airavata/credential/store/credential/impl/ssh/SSHCredentialGenerator.java <https://reviews.apache.org/r/15789/#comment56480> We decided to generate pass phrase based on DN and a random value. So the pass phrase = HASH(DN + randomvalue). We only store randomvalue in credential store. https://svn.apache.org/repos/asf/airavata/trunk/modules/rest/service/src/main/java/org/apache/airavata/services/registry/rest/resources/CredentialRegistryResource.java <https://reviews.apache.org/r/15789/#comment56485> token should not be user id. https://svn.apache.org/repos/asf/airavata/trunk/modules/airavata-client/src/main/java/org/apache/airavata/client/api/CredentialStoreManager.java <https://reviews.apache.org/r/15789/#comment56476> I am not sure why do you want to get credentials through the API. I dont think its a good idea to expose a function to get credentials through API. https://svn.apache.org/repos/asf/airavata/trunk/modules/airavata-client/src/main/java/org/apache/airavata/client/api/CredentialStoreManager.java <https://reviews.apache.org/r/15789/#comment56477> persistCredentials is a more appropriate name (?). To create credentials we need more information. (Mainly when storing them in the credential store). E.g :- admin username, admin email. In addition to public key we also need to include "fromIp" like configuration. (To restrict access resource) https://svn.apache.org/repos/asf/airavata/trunk/modules/airavata-client/src/main/java/org/apache/airavata/client/api/CredentialStoreManager.java <https://reviews.apache.org/r/15789/#comment56478> same comments apply here as previous https://svn.apache.org/repos/asf/airavata/trunk/modules/airavata-client/src/main/java/org/apache/airavata/client/impl/CredentialStoreManagerImpl.java <https://reviews.apache.org/r/15789/#comment56479> As mentioned in a previous comment we should not expose credentials through API. https://svn.apache.org/repos/asf/airavata/trunk/modules/registry/airavata-jpa-registry/src/main/java/org/apache/airavata/persistance/registry/jpa/impl/AiravataJPARegistry.java <https://reviews.apache.org/r/15789/#comment56481> Again not a fan of having this method. https://svn.apache.org/repos/asf/airavata/trunk/modules/registry/airavata-jpa-registry/src/main/java/org/apache/airavata/persistance/registry/jpa/impl/AiravataJPARegistry.java <https://reviews.apache.org/r/15789/#comment56482> token should not be username. There is a method to generate token id in credential store. https://svn.apache.org/repos/asf/airavata/trunk/modules/registry/airavata-jpa-registry/src/main/java/org/apache/airavata/persistance/registry/jpa/impl/AiravataJPARegistry.java <https://reviews.apache.org/r/15789/#comment56483> log and throw exception. Applies to other places also https://svn.apache.org/repos/asf/airavata/trunk/modules/registry/registry-api/src/main/java/org/apache/airavata/registry/api/CredentialRegistry.java <https://reviews.apache.org/r/15789/#comment56484> All comments mentioned earlier also apply here. Mainly credential retrieval. - Thejaka Amila Kanewala On Nov. 22, 2013, 7:43 p.m., Viknes Balasubramanee wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15789/ > ----------------------------------------------------------- > > (Updated Nov. 22, 2013, 7:43 p.m.) > > > Review request for Airavata. > > > Bugs: Airavata-952 > https://issues.apache.org/jira/browse/Airavata-952 > > > Repository: Airavata > > > Description > ------- > > Add credential store functionalities to the Airavata API > > > Diffs > ----- > > > https://svn.apache.org/repos/asf/airavata/trunk/modules/airavata-client/src/main/java/org/apache/airavata/client/AiravataClient.java > 1541741 > > https://svn.apache.org/repos/asf/airavata/trunk/modules/airavata-client/src/main/java/org/apache/airavata/client/api/AiravataAPI.java > 1541741 > > https://svn.apache.org/repos/asf/airavata/trunk/modules/airavata-client/src/main/java/org/apache/airavata/client/api/CredentialStoreManager.java > PRE-CREATION > > https://svn.apache.org/repos/asf/airavata/trunk/modules/airavata-client/src/main/java/org/apache/airavata/client/impl/CredentialStoreManagerImpl.java > PRE-CREATION > > https://svn.apache.org/repos/asf/airavata/trunk/modules/credential-store/pom.xml > 1541741 > > https://svn.apache.org/repos/asf/airavata/trunk/modules/credential-store/src/main/java/org/apache/airavata/credential/store/credential/impl/ssh/SSHCredential.java > PRE-CREATION > > https://svn.apache.org/repos/asf/airavata/trunk/modules/credential-store/src/main/java/org/apache/airavata/credential/store/credential/impl/ssh/SSHCredentialGenerator.java > PRE-CREATION > > https://svn.apache.org/repos/asf/airavata/trunk/modules/credential-store/src/main/java/org/apache/airavata/credential/store/store/impl/SSHCredentialWriter.java > PRE-CREATION > > https://svn.apache.org/repos/asf/airavata/trunk/modules/credential-store/src/main/java/org/apache/airavata/credential/store/store/impl/db/CredentialsDAO.java > 1541741 > > https://svn.apache.org/repos/asf/airavata/trunk/modules/registry/airavata-jpa-registry/pom.xml > 1541741 > > https://svn.apache.org/repos/asf/airavata/trunk/modules/registry/airavata-jpa-registry/src/main/java/org/apache/airavata/persistance/registry/jpa/impl/AiravataJPARegistry.java > 1541741 > > https://svn.apache.org/repos/asf/airavata/trunk/modules/registry/registry-api/src/main/java/org/apache/airavata/registry/api/AiravataRegistry2.java > 1541741 > > https://svn.apache.org/repos/asf/airavata/trunk/modules/registry/registry-api/src/main/java/org/apache/airavata/registry/api/CredentialRegistry.java > PRE-CREATION > > https://svn.apache.org/repos/asf/airavata/trunk/modules/rest/client/src/main/java/org/apache/airavata/rest/client/CredentialStoreResourceClient.java > PRE-CREATION > > https://svn.apache.org/repos/asf/airavata/trunk/modules/rest/client/src/main/java/org/apache/airavata/rest/client/RegistryClient.java > 1541741 > > https://svn.apache.org/repos/asf/airavata/trunk/modules/rest/mappings/src/main/java/org/apache/airavata/rest/mappings/utils/ResourcePathConstants.java > 1541741 > > https://svn.apache.org/repos/asf/airavata/trunk/modules/rest/service/src/main/java/org/apache/airavata/services/registry/rest/resources/CredentialRegistryResource.java > PRE-CREATION > > https://svn.apache.org/repos/asf/airavata/trunk/modules/rest/service/src/main/java/org/apache/airavata/services/registry/rest/resources/UserRegistryResource.java > 1541741 > > Diff: https://reviews.apache.org/r/15789/diff/ > > > Testing > ------- > > > Thanks, > > Viknes Balasubramanee > >
