Add option to set alternative ccname env var #124
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
simo5
requested changes
Feb 4, 2017
Contributor
simo5
left a comment
simo5
left a comment
There was a problem hiding this comment.
I am ok with the PR, just change the one detail I mentioned.
src/environ.c
Outdated
| "KRB5CCNAME file (%s) lookup failed!", path); | ||
| } | ||
|
|
||
| char *name = cfg->ccname_envvar != NULL |
Contributor
There was a problem hiding this comment.
Can you please set the default env var in ccname_envar when the cgf is allocated and avoid this if/else ?
Contributor
Author
|
On Sat, Feb 04, 2017 at 01:28:53AM -0800, Simo Sorce wrote:
simo5 requested changes on this pull request.
I am ok with the PR, just change the one detail I mentioned.
> @@ -286,8 +286,11 @@ static void mag_set_KRB5CCNAME(request_rec *req, struct mag_config *cfg,
"KRB5CCNAME file (%s) lookup failed!", path);
}
+ char *name = cfg->ccname_envvar != NULL
Can you please set the default env var in ccname_envar when the cgf is allocated and avoid this if/else ?
Sure thing @simo5. Will update the PR next week.
… --
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
https://github.com/modauthgssapi/mod_auth_gssapi/pull/124#pullrequestreview-20145158
|
In some cases (e.g. if you want to convey the ccname over AJP) the request environment variable name "KRB5CCNAME" is not appropriate. Add the GssapiDelegCcacheEnvVar option that allows the env var name to be changed. Fixes: https://github.com/modauthgssapi/mod_auth_gssapi/issues/123
frasertweedale
force-pushed
the
feature/ccname-env-var
branch
from
34c370a to
dea30c9
Compare
Contributor
Author
|
@Simo are you happy with this patch now? |
Contributor
|
Yes, I'll fix a style issue as I push it, it'd be nice to have a test to make sure we do not regress here, and the proper variable is populated, you can open a spearate PR for that though |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In some cases (e.g. if you want to convey the ccname over AJP) the
request environment variable name "KRB5CCNAME" is not appropriate.
Add the GssapiDelegCcacheEnvVar option that allows the env var name
to be changed.
Fixes: https://github.com/modauthgssapi/mod_auth_gssapi/issues/123