[
https://issues.apache.org/jira/browse/DIRKRB-692?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16355619#comment-16355619
]
Fabiano Tarlao edited comment on DIRKRB-692 at 2/7/18 3:53 PM:
---------------------------------------------------------------
I cannot provide a final test for your project, I have not understood how your
integration tests work. I think you spawn a local kdc instance with predefined
domain users.
I have provided a normal test KrbClientTest.java that assumes a kdc is already
running somewhere and KrbClientTest.java class contains constants that should
be initialized properly...
It also loads the kdc configuration from an available kerb5.conf. You can also
change that part to a getResource... + new File(...getURI)... It depends on
your best practises about. Hope useful
Regards
was (Author: ftarlao):
I cannot provide a final test for your project, I have not understood how your
integration tests work. I think you spawn a local kdc instance with predefined
domain users.
I have provided a normal test KrbClientTest.java that assumes a kdc is already
running somewhere and that class has constants that should be initialized
properly...
[|https://issues.apache.org/jira/secure/DeleteAttachment!default.jspa?id=13136579&deleteAttachmentId=12909628&from=issue]it
also loads the kdc configuration from an available kerb5.conf. You can also
change that part to a getResource... new File(...getURI)... I have currently
no-idea of your best practises about. Hope useful
Regards
> 1)sgtTicket clientPrincipal is not initialized + 2)KrbClient fails to store
> SGT ticket in cache file
> ----------------------------------------------------------------------------------------------------
>
> Key: DIRKRB-692
> URL: https://issues.apache.org/jira/browse/DIRKRB-692
> Project: Directory Kerberos
> Issue Type: Bug
> Affects Versions: 1.1.0
> Environment: Linux Mint 17.1 + Netbeans 8.1 with a Maven Project +
> kerb-core 1.1.0 in Windows AD (KDC is Windows 2016 Server)
> Reporter: Fabiano Tarlao
> Priority: Major
> Labels: easyfix
> Attachments: KrbClientTest.java, bug1.diff, bug2.diff
>
>
> Two bugs that interact each other
> h1. 1)
> *SgtTicket*, returned by *KrbClient.requestSgt(..)*, has a _null_
> *clientPrincipal* field (unassigned). Perhaps this is not mandatory but your
> code assumes this property is populated (see later). I highly suggest to
> populate this field.
> I have wrote a workaround that overrides the *requestSgt* method and works
> for the *USE_TGT* case:
>
> {code:java}
> @Override
> public SgtTicket requestSgt(KOptions requestOptions) throws KrbException {
> SgtTicket requestSgt = super.requestSgt(requestOptions);
> TgtTicket tgt = (TgtTicket)
> requestOptions.getOptionValue(KrbOption.USE_TGT);
> if(tgt != null){
> requestSgt.setClientPrincipal(tgt.getClientPrincipal());
> }
> return requestSgt;
> }{code}
>
> h1. 2)
> Creating a new credential cache file when storing only one SGT (service
> ticket) fails. (i.e., trying to create a new cache file containing only one
> SGT and no TGT)
> Invoking *KrbClient.storeTicket(sgtTicket, File)* fails for this reason (here
> is the original code in *KrbClientBase* class, my comments in RED ):
> {{public void storeTicket(SgtTicket sgtTicket, File ccacheFile) throws
> KrbException {}}
> {{ LOG.info("Storing the sgt to the credential cache file.");}}
> {{ if (!ccacheFile.exists()) {}}
> {{ createCacheFile(ccacheFile);{color:#ff0000} //Correctly
> creates a new file but...{color}}}
> {{ if (ccacheFile.exists() && ccacheFile.canWrite()) {}}
> {{ CredentialCache cCache = new CredentialCache();}}
> {{ try {}}
> {{ cCache.load(ccacheFile);{color:#ff0000} //..this line
> EXPLODES cause it tries to initialize from an empty file, the unexistent file
> case is not managed correctly{color}}}
> {{ cCache.addCredential(new Credential(sgtTicket,
> sgtTicket.getClientPrincipal()));}}
> {{
> cCache.setPrimaryPrincipal(sgtTicket.getClientPrincipal());}}
> {{ cCache.store(ccacheFile);}}
> {{ } catch (IOException e) {}}
> {{ throw new KrbException("Failed to store sgt", e);}}
> {{ } else {}}
> {{ throw new IllegalArgumentException("Invalid ccache file, "}}
> {{ + "not exist or writable: " +
> ccacheFile.getAbsolutePath());}}
> {{}}}
> Here follows my proposal/fix, this code correctly manages the MIT ccache file
> creation for one SGT, please note that this fix assumes that bug 1 is already
> fixed:
> {{public static void storeTicket(SgtTicket sgtTicket, File ccacheFile) throws
> KrbException {}}
> {{ LOG.info("Storing the sgt to the credential cache file.");}}
> {{ boolean isFreshNew = !ccacheFile.exists();}}
> {{ if (isFreshNew) {}}
> {{ createCacheFile(ccacheFile);}}
> {{ if (ccacheFile.exists() && ccacheFile.canWrite()) {}}
> {{ }}
> {{ try {}}
> {{ CredentialCache cCache;}}
> {{ if(!isFreshNew){}}
> {{ cCache = new CredentialCache(sgtTicket);
> {color:#ff0000}//This constructor sets also the cCache principal from
> sgtTicket principal{color}}}
> {{ cCache.load(ccacheFile);}}
> {{ cCache.addCredential(new Credential(sgtTicket,
> sgtTicket.getClientPrincipal()));}}
> {{
> cCache.setPrimaryPrincipal(sgtTicket.getClientPrincipal());}}
> {{ } else {}}
> {{ cCache = new CredentialCache(sgtTicket);}}
> {{ cCache.store(ccacheFile);}}
> {{ } catch (IOException e) {}}
> {{ throw new KrbException("Failed to store sgt", e);}}
> {{ } else {}}
> {{ throw new IllegalArgumentException("Invalid ccache file, "}}
> {{ + "not exist or writable: " +
> ccacheFile.getAbsolutePath());}}
> Please note that *YOUR CredentialCache contructor assumes the clientPrincipal
> is populated* ;)
> Hope useful,
> regards
> Fabiano
> P.s: Attached two paches for the project, bug2.diff for bug 2 is..*I think..
> production ready*.
> bug1.diff for bug 1 is untested.. I ported the modification of my overriding
> method to the *AbstractInternalKrbClient.requestSgt()* method. But I have not
> performed run/test of modules, your project is big and needs time to manage
> well. *BUT* is this the right solution? or SHOULD the *clientPrincipal* be
> already populated from the doRequestSgt method? I suggest to look for a
> solution at a deeper level starting from *DefaultInternalKrbClient*. Consider
> bug1.diff a fair workaround.
> P.s.2:Added a pull request on GitHub
> Regards
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
