You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by GitBox <gi...@apache.org> on 2020/07/21 16:05:39 UTC
[GitHub] [directory-kerby] liujiayi771 opened a new pull request #49: DIRKRB-745
liujiayi771 opened a new pull request #49:
URL: https://github.com/apache/directory-kerby/pull/49
Add command tool ktadd(xst) in remote kadmin tool.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-kerby] plusplusjiajia commented on a change in pull request #49: DIRKRB-745
Posted by GitBox <gi...@apache.org>.
plusplusjiajia commented on a change in pull request #49:
URL: https://github.com/apache/directory-kerby/pull/49#discussion_r467597390
##########
File path: kerby-kerb/kerb-admin-server/src/main/java/org/apache/kerby/kerberos/kerb/admin/server/kadmin/AdminServerHandler.java
##########
@@ -22,23 +22,19 @@
import org.apache.kerby.kerberos.kerb.KrbException;
import org.apache.kerby.kerberos.kerb.admin.kadmin.local.LocalKadmin;
import org.apache.kerby.kerberos.kerb.admin.kadmin.local.LocalKadminImpl;
-import org.apache.kerby.kerberos.kerb.admin.message.AddPrincipalRep;
-import org.apache.kerby.kerberos.kerb.admin.message.AdminMessage;
-import org.apache.kerby.kerberos.kerb.admin.message.AdminMessageCode;
-import org.apache.kerby.kerberos.kerb.admin.message.AdminMessageType;
-import org.apache.kerby.kerberos.kerb.admin.message.DeletePrincipalRep;
-import org.apache.kerby.kerberos.kerb.admin.message.GetprincsRep;
-import org.apache.kerby.kerberos.kerb.admin.message.KadminCode;
-import org.apache.kerby.kerberos.kerb.admin.message.RenamePrincipalRep;
Review comment:
It's better not to use * for import.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-kerby] plusplusjiajia commented on pull request #49: DIRKRB-745
Posted by GitBox <gi...@apache.org>.
plusplusjiajia commented on pull request #49:
URL: https://github.com/apache/directory-kerby/pull/49#issuecomment-677078729
LGTM, I've merged this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-kerby] coheigea commented on pull request #49: DIRKRB-745
Posted by GitBox <gi...@apache.org>.
coheigea commented on pull request #49:
URL: https://github.com/apache/directory-kerby/pull/49#issuecomment-677291818
@plusplusjiajia Why not just merge the PR via the "Merge pull request" button on this page?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-kerby] dwalluck commented on a change in pull request #49: DIRKRB-745
Posted by GitBox <gi...@apache.org>.
dwalluck commented on a change in pull request #49:
URL: https://github.com/apache/directory-kerby/pull/49#discussion_r467636831
##########
File path: kerby-kerb/kerb-admin-server/src/main/java/org/apache/kerby/kerberos/kerb/admin/server/kadmin/AdminServerHandler.java
##########
@@ -199,6 +199,42 @@ private ByteBuffer handleGetprincsReq(LocalKadmin localKadmin, XdrFieldInfo[] fi
return responseError;
}
}
+
+ private ByteBuffer handleExportKeytabReq(LocalKadmin localKadmin, XdrFieldInfo[] fieldInfos) throws IOException {
+ String principals = ((String) fieldInfos[2].getValue());
+
+ if (principals != null) {
+ List<String> princList = stringToList(principals);
+ if (princList.size() != 0) {
+ LOG.info("Exporting keytab file for " + principals + "...");
+ File path = new File("/tmp/" + System.currentTimeMillis());
Review comment:
I just happened to notice this. It is not the correct temporary directory on Windows and it is also not secure. Consider using `File.createTempFile()` instead.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-kerby] plusplusjiajia commented on a change in pull request #49: DIRKRB-745
Posted by GitBox <gi...@apache.org>.
plusplusjiajia commented on a change in pull request #49:
URL: https://github.com/apache/directory-kerby/pull/49#discussion_r471870040
##########
File path: kerby-kerb/kerb-admin-server/src/test/java/org/apache/kerby/kerberos/kerb/admin/server/RemoteKadminTest.java
##########
@@ -0,0 +1,236 @@
+package org.apache.kerby.kerberos.kerb.admin.server;
Review comment:
The apache license should be added.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-kerby] coheigea commented on pull request #49: DIRKRB-745
Posted by GitBox <gi...@apache.org>.
coheigea commented on pull request #49:
URL: https://github.com/apache/directory-kerby/pull/49#issuecomment-677372592
Ah I see. You need to follow the instructions here about linking your ASF + Github accounts: https://gitbox.apache.org/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-kerby] coheigea closed pull request #49: DIRKRB-745
Posted by GitBox <gi...@apache.org>.
coheigea closed pull request #49:
URL: https://github.com/apache/directory-kerby/pull/49
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-kerby] liujiayi771 commented on a change in pull request #49: DIRKRB-745
Posted by GitBox <gi...@apache.org>.
liujiayi771 commented on a change in pull request #49:
URL: https://github.com/apache/directory-kerby/pull/49#discussion_r468343736
##########
File path: kerby-kerb/kerb-admin-server/src/main/java/org/apache/kerby/kerberos/kerb/admin/server/kadmin/AdminServerHandler.java
##########
@@ -199,6 +199,42 @@ private ByteBuffer handleGetprincsReq(LocalKadmin localKadmin, XdrFieldInfo[] fi
return responseError;
}
}
+
+ private ByteBuffer handleExportKeytabReq(LocalKadmin localKadmin, XdrFieldInfo[] fieldInfos) throws IOException {
+ String principals = ((String) fieldInfos[2].getValue());
+
+ if (principals != null) {
+ List<String> princList = stringToList(principals);
+ if (princList.size() != 0) {
+ LOG.info("Exporting keytab file for " + principals + "...");
+ File path = new File("/tmp/" + System.currentTimeMillis());
Review comment:
I have fixed it by using Files.createTempDirectory("ktadd").
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-kerby] plusplusjiajia commented on pull request #49: DIRKRB-745
Posted by GitBox <gi...@apache.org>.
plusplusjiajia commented on pull request #49:
URL: https://github.com/apache/directory-kerby/pull/49#issuecomment-677311698
> @plusplusjiajia Why not just merge the PR via the "Merge pull request" button on this page?
@coheigea I can't see the "Merge pull request" button and "close" button, I think I don't have these permissions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-kerby] plusplusjiajia commented on pull request #49: DIRKRB-745
Posted by GitBox <gi...@apache.org>.
plusplusjiajia commented on pull request #49:
URL: https://github.com/apache/directory-kerby/pull/49#issuecomment-677383746
> Ah I see. You need to follow the instructions here about linking your ASF + Github accounts: https://gitbox.apache.org/
Thanks for your guide. :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
[GitHub] [directory-kerby] liujiayi771 commented on a change in pull request #49: DIRKRB-745
Posted by GitBox <gi...@apache.org>.
liujiayi771 commented on a change in pull request #49:
URL: https://github.com/apache/directory-kerby/pull/49#discussion_r468344132
##########
File path: kerby-kerb/kerb-admin-server/src/main/java/org/apache/kerby/kerberos/kerb/admin/server/kadmin/AdminServerHandler.java
##########
@@ -22,23 +22,19 @@
import org.apache.kerby.kerberos.kerb.KrbException;
import org.apache.kerby.kerberos.kerb.admin.kadmin.local.LocalKadmin;
import org.apache.kerby.kerberos.kerb.admin.kadmin.local.LocalKadminImpl;
-import org.apache.kerby.kerberos.kerb.admin.message.AddPrincipalRep;
-import org.apache.kerby.kerberos.kerb.admin.message.AdminMessage;
-import org.apache.kerby.kerberos.kerb.admin.message.AdminMessageCode;
-import org.apache.kerby.kerberos.kerb.admin.message.AdminMessageType;
-import org.apache.kerby.kerberos.kerb.admin.message.DeletePrincipalRep;
-import org.apache.kerby.kerberos.kerb.admin.message.GetprincsRep;
-import org.apache.kerby.kerberos.kerb.admin.message.KadminCode;
-import org.apache.kerby.kerberos.kerb.admin.message.RenamePrincipalRep;
Review comment:
I have fixed it, and I also added the UT of the remote kadmin server.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org