SUBMARINE-646. Create rest api to authenticate user from LDAP #419
Conversation
|
|
||
| private void authenticate(String username, String password) throws Exception { | ||
| DirContext ctx = null; | ||
| Hashtable<String, String> HashEnv = new Hashtable<>(); |
There was a problem hiding this comment.
| Hashtable<String, String> HashEnv = new Hashtable<>(); | |
| Hashtable<String, String> hashEnv = new Hashtable<>(); |
xunliu
changed the title
xunliu
changed the title
| ctx = new InitialDirContext(HashEnv); | ||
| } | ||
| catch (AuthenticationException e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
Please use LOG.error(xxx) to print exception.
| @POST | ||
| @Produces(MediaType.APPLICATION_JSON) | ||
| @Consumes(MediaType.APPLICATION_FORM_URLENCODED) | ||
| public Response authenticateUser(@FormParam("username") String username, |
There was a problem hiding this comment.
Please change param name and value nameusername to userName.
| ctx.close(); | ||
| } | ||
| catch (NamingException e) { | ||
| LOG.error(e.getStackTrace().toString()); |
There was a problem hiding this comment.
Please change to LOG.error(e.getMessage(), e);
There was a problem hiding this comment.
Thanks @Eroschang for the work. I found some redundant blank lines should be removed.
submarine-server/server-core/pom.xml
Outdated
| <dependency> | ||
| <groupId>io.jsonwebtoken</groupId> | ||
| <artifactId>jjwt-api</artifactId> | ||
| <version>0.11.2</version> |
There was a problem hiding this comment.
All the dependencies should be moved to project pom.xml and use the defined version property.
| } | ||
| catch (AuthenticationException e) { |
There was a problem hiding this comment.
code style
| } | |
| catch (AuthenticationException e) { | |
| } catch (AuthenticationException e) { |
| } | ||
| catch (NamingException e) { |
There was a problem hiding this comment.
| } | |
| catch (NamingException e) { | |
| } catch (NamingException e) { |
| } | ||
|
|
||
| private String createJWT(String id, long ttlMillis){ | ||
|
|
There was a problem hiding this comment.
@Eroschang Please delete this blank line.
| package org.apache.submarine.server.api.ldap; | ||
|
|
||
| public class Ldap { | ||
| private String username; |
There was a problem hiding this comment.
Please change username to userName.
| private String username; | ||
| private String password; | ||
|
|
||
| public String getUsername() { |
There was a problem hiding this comment.
Please change getUsername to getUserName.
| return username; | ||
| } | ||
|
|
||
| public void setUsername(String username) { |
There was a problem hiding this comment.
Please change setUsername(String username) to setUserName(String userName).
|
|
||
| private static final Logger LOG = LoggerFactory.getLogger(LdapAuthenticateRestApi.class); | ||
|
|
||
| private void authenticate(String username, String password) throws Exception { |
There was a problem hiding this comment.
Please change username to userName.
| return password; | ||
| } | ||
|
|
||
| public void setUserName(String user_name) { |
There was a problem hiding this comment.
function setUserName param is user_name, this.userName = userName; corrent??
What is this PR for?
Create rest api to authenticate user from LDAP server.
The server should be provided by manager now.
What type of PR is it?
Improvement
Todos
What is the Jira issue?
https://issues.apache.org/jira/browse/SUBMARINE-646
How should this be tested?
https://travis-ci.org/github/Eroschang/submarine/builds/731207490
Screenshots (if appropriate)
Questions: