Configure a code for Authenticators to indicate AUTH_USER_NOT_FOUND#68
Configure a code for Authenticators to indicate AUTH_USER_NOT_FOUND#68carrvo wants to merge 6 commits intophokz:masterfrom
Conversation
Original commit from https://github.com/mkj return AUTH_USER_NOT_FOUND
keep the existing behaviour (no special return codes) as the default, but allow the user to define a particular return code (new directive) to mean the user does not exist.
There was a problem hiding this comment.
Looks pretty good! Just needs a few comment/naming tweaks for clarity.
Side note: I used the new GitHub review experience to submit the comments, and I see they are all jumbled up here. 😓 They might make more sense if you use the 'Files Changed' tab at the top to see them in order in the file 😅
| return AUTH_GRANTED; | ||
| } | ||
|
|
||
| /* Nonexistant login or (for some configurations) incorrect password |
There was a problem hiding this comment.
| /* Nonexistant login or (for some configurations) incorrect password | |
| /* Determine whether the return code means 'non-existent user' or 'incorrect |
| char *context; /* Context string from AuthExternalContext */ | ||
| int groupsatonce; /* Check all groups in one call? */ | ||
| int providecache; /* Provide auth data to mod_authn_socache? */ | ||
| int authn_no_user_code; /* External code to use for no user (HTTP 401) */ |
There was a problem hiding this comment.
The variable name should probably be changed to something about user not found and the description needs to be fixed.
AUTH_USER_NOT_FOUND does not mean to send 401 and re-authenticate, though that can be the final result. It means that this particular authn module cannot find the submitted user, and Apache should try the next authn module (if the admin has defined a next module to check)
| dir->context = NULL; /* no default */ | ||
| dir->groupsatonce = 1; /* default to on */ | ||
| dir->providecache = 0; /* default to off */ | ||
| dir->authn_no_user_code = 0; /* default to 0 to ignore */ |
There was a problem hiding this comment.
see comment for line 84
| (void *)APR_OFFSETOF(authnz_external_dir_config_rec, authn_no_user_code), | ||
| OR_AUTHCFG, | ||
| "Set to a return code that the authenticator uses to indicate that the " | ||
| "user is not found (respond with HTTP 401). Set to 0 to ignore."), |
There was a problem hiding this comment.
see comment for line 84
| } | ||
|
|
||
| /* Nonexistant login or (for some configurations) incorrect password | ||
| * Handle this differently so that unknown users can be passed to the next |
There was a problem hiding this comment.
| * Handle this differently so that unknown users can be passed to the next | |
| * password'. If 'non-existent user', move on to the next external authenticator |
| /* Nonexistant login or (for some configurations) incorrect password | ||
| * Handle this differently so that unknown users can be passed to the next | ||
| * Apache AuthBasicProvider | ||
| * Note that a configuration of 0, this will always be true and thus ignored */ |
There was a problem hiding this comment.
might be better to move this line to documentation, and rephrase to something about "0 is reserved for successful authorization" or something like that. It's not so much 'ignored' as it is that this code never even runs (success already returned)
What are your thoughts?
| * Handle this differently so that unknown users can be passed to the next | ||
| * Apache AuthBasicProvider | ||
| * Note that a configuration of 0, this will always be true and thus ignored */ | ||
| if (code != dir->authn_no_user_code) |
There was a problem hiding this comment.
see comment for line 84
| "AuthExtern %s [%s]: Failed (%d) for user %s", | ||
| extname, extpath, code, r->user); | ||
| } | ||
|
|
There was a problem hiding this comment.
| /* If all external authenticators return 'non-existent user', we return | |
| * AUTH_USER_NOT_FOUND so that unknown users can be passed to the next | |
| * Apache AuthBasicProvider */ |
| directive. In Apache 2.4, the notion of authoritativeness is | ||
| thankfully almost entirely gone, so this directive is too. | ||
|
|
||
| If you want the ability to specify a return code for your authenticator |
There was a problem hiding this comment.
| If you want the ability to specify a return code for your authenticator | |
| If you want to allow your authenticator to gracefully hand off to the |
| thankfully almost entirely gone, so this directive is too. | ||
|
|
||
| If you want the ability to specify a return code for your authenticator | ||
| to indicate that it could not find a user: |
There was a problem hiding this comment.
| to indicate that it could not find a user: | |
| next apache authentication module when your authenticator cannot locate | |
| the submitted user, you have the ability to specify a return code for | |
| your authenticator to indicate that it could not find a user: |
|
@carrvo, I sent you an e-mail at the address in your profile. |
3 participants
Resolves #40
Add AuthnUserNotFoundCode directive
Keeps the existing behaviour (no special return codes) as the default, but allow the user to define a particular return code (new directive) to mean the user does not exist.