Skip to content

Conversation

@thalman
Copy link
Contributor

@thalman thalman commented Dec 3, 2025

SSSD should refuse to start when SSSD is in server mode and full_name_format is set to %1s$.

SSSD in server mode on IPA server is not allowed to use
short names.
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly implements a check to prevent SSSD from starting in IPA server mode when full_name_format is set to use only short names. The accompanying test case is also well-written and verifies the new behavior. My only suggestion is to add a system log message for the startup failure to improve visibility for administrators, as the current implementation only logs to the debug log.

Comment on lines +1488 to +1492
if (strcmp(be_ctx->domain->names->fq_fmt, "%1$s") == 0) {
DEBUG(SSSDBG_FATAL_FAILURE, "%s is set to [%s], this is prohibited in server mode!\n",
CONFDB_FULL_NAME_FORMAT, be_ctx->domain->names->fq_fmt);
return EINVAL;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

When SSSD fails to start due to a configuration error, it's crucial to log a message to the system log (e.g., syslog or journald) so that administrators can easily diagnose the problem without needing to enable high-level debug logging. The existing check for a non-default full_name_format already does this using sss_log(). This new fatal error should do the same.

    if (strcmp(be_ctx->domain->names->fq_fmt, "%1$s") == 0) {
        DEBUG(SSSDBG_FATAL_FAILURE, "%s is set to [%s], this is prohibited in server mode!\n",
              CONFDB_FULL_NAME_FORMAT, be_ctx->domain->names->fq_fmt);
        sss_log(SSS_LOG_ERR, "%s is set to [%s], this is prohibited in server mode!\n",
                CONFDB_FULL_NAME_FORMAT, be_ctx->domain->names->fq_fmt);
        return EINVAL;
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants