fix: Improve SAML configuration checks and update warning messages

[ktyagiapphelix2u]

Description

Improve SAML command to stop wrong warnings. Tests updated too.

Private Ticket

https://2u-internal.atlassian.net/jira/software/c/projects/BOMS/boards/3017?assignee=712020%3A61c35560-3472-4e12-b833-884e5c4bbff4&selectedIssue=BOMS-64

Related PR

#37330

Saml Configuration Report

SAML Configuration Check Report
==================================================

[WARNING] Provider (id=74, name=, slug=default-apphelix, site_id=1) has outdated SAML config (id=50) which should be updated to the current SAML config (id=77).
[INFO] Provider (id=74, name=, slug=default-apphelix, site_id=1) has SAML config (id=50, slug='default-mock') that does not match the provider's slug.
[WARNING] Provider (id=77, name=, slug=default-melody, site_id=1, enabled=False) has outdated SAML config (id=50) which should be updated to the current SAML config (id=77).
[INFO] Provider (id=77, name=, slug=default-melody, site_id=1, enabled=False) has SAML config (id=50, slug='default-mock') that does not match the provider's slug.
[WARNING] Provider (id=78, name=, slug=default-chintan, site_id=1, enabled=False) has outdated SAML config (id=50) which should be updated to the current SAML config (id=77).
[INFO] Provider (id=78, name=, slug=default-chintan, site_id=1, enabled=False) has SAML config (id=50, slug='default-mock') that does not match the provider's slug.
[INFO] Provider (id=88, name=, slug=vaibhav1, site_id=1) has SAML config (id=61, slug='apphelix1') that does not match the provider's slug.
[INFO] Provider (id=89, name=, slug=vaibhav2, site_id=1) has SAML config (id=61, slug='apphelix1') that does not match the provider's slug.
[WARNING] Provider (id=117, name=, slug=default-slug, site_id=6, enabled=False) has SAML config (id=73, enabled=False).
[WARNING] Provider (id=117, name=, slug=default-slug, site_id=6, enabled=False) SAML config (id=73, site_id=1) does not match the provider's site_id.
[INFO] Provider (id=117, name=, slug=default-slug, site_id=6, enabled=False) has SAML config (id=73, slug='slug', enabled=False) that does not match the provider's slug.
[WARNING] Provider (id=119, name=, slug=slug, site_id=1, enabled=False) has SAML config (id=76, enabled=False).
[WARNING] Provider (id=119, name=, slug=slug, site_id=1, enabled=False) SAML config (id=76, site_id=8) does not match the provider's site_id.
[WARNING] Provider (id=123, name=, slug=default-edx, site_id=1, enabled=False) has SAML config (id=79, enabled=False).
[INFO] Provider (id=129, name=, slug=defaultssss, site_id=1) has SAML config (id=82, slug='default-s') that does not match the provider's slug.
[INFO] Provider (id=132, name=, slug=diffrent-slug, site_id=4) has SAML config (id=84, slug='test-config') that does not match the provider's slug.
[INFO] Provider (id=137, name=, slug=default0ssd, site_id=1) has SAML config (id=90, slug='default-app') that does not match the provider's slug.
[WARNING] Provider (id=143, name=, slug=pip, site_id=9, enabled=False) has no direct SAML configuration and no matching default configuration was found.
[WARNING] Provider (id=144, name=Provider I, slug=myslug, site_id=10) has no direct SAML configuration and no matching default configuration was found.
[WARNING] Provider (id=145, name=Provider 2, slug=mysecondslug, site_id=10) has no direct SAML configuration and no matching default configuration was found.
[WARNING] Provider (id=146, name=Provider 3, slug=myslug2, site_id=11) has no direct SAML configuration and no matching default configuration was found.
CHECK SUMMARY:
  Providers checked: 22

Informational only:
  Slug mismatches: 9
  Missing configs: 4

Issues requiring attention:
  Outdated: 3
  Site mismatches: 2
  Disabled configs: 3
  Errors: 0

Total issues requiring attention: 8

[ktyagiapphelix2u]

why i change get_config()

when use get_config(), it show many error, same error again again for missing metadata.
real config warning lost in many “No SAMLProviderData found” message.
this kind of Error: 2025-09-23 09:49:45,793 ERROR 339 [common.djangoapps.third_party_auth.models] [user None] [ip None] models.py:873 - No SAMLProviderData found for provider "" with entity id "" and IdP slug "test-config". Run "manage.py saml pull" to fix or debug.
It was counting metadata missing as error, but it not real config problem.
this was checking config and metadata both. But this command should only check config. Metadata is for --pull command. this was the mix thing which making me confused.

So I have improved it to new logic
I am doing same check, do same config check like get_config(), but no spam error message.
It check direct config and default config like get_config() do. It only say warning if both direct and default config missing.
First check direct, then default, Now config check and metadata check not getting mix

[ktyagiapphelix2u]

The Problem with get_config():

get_config() was mixing configuration validation with metadata validation. It was generating spam error messages about missing metadata ("No SAMLProviderData found"). It was checking both config AND metadata, but this command should only check config. Metadata checking belongs in the --pull command, not the config check command

[robrap]

Proposed summary output:

CHECK SUMMARY:
  Providers checked: 17

Informational only:
  Slug mismatches: 10
  Missing configs: 0

Issues requiring attention:
  Outdated: 3
  Site mismatches: 2
  Errors: 0

Total issues requiring attention: 5

[ktyagiapphelix2u]

@robrap I have updated the test cases and updated the saml config report for missing config one and kept it as warning only and added test case when it will give missing config

Create the provider
Add a new provider:
site: select lost.com
slug: enter my-slug
SAML configuration: leave blank (do not select anything)
Save.

Check SAML configurations

Filter by site lost.com.
Make sure there is NO SAMLConfiguration with [slug='default' for lost.com.
If one exists, either delete it or change its slug to something else (not default)

this will give missing config as [WARNING] Provider (id=146, name=Provider 3, slug=myslug2, site_id=11) has no direct SAML configuration and no matching default configuration was found. message.

[robrap]

I didn't go through tests yet. This is a start. Thank you.

[robrap]

Some comments to help progress. I'm not sure what to do because we are clearly spinning on this PR, and there is a lot of code (especially test code) for something that we have lived without. On the one hand, I'd like to land this without putting in too much time. On the other hand, I don't want to dump in a bunch of a new code that needs to be maintained forever that isn't serving a real benefit.

It may make sense to simple drop the observability calls to simplify the code and tests a little. The point is that someone will run the command and you'll just use the output that gets reported. There will still be redundancy in the tests that could possibly get cleaned up, but this would remove some of it.

[chintanjoshi-apphelix-2u]

Comment can be removed.

[robrap]

[inform] I did ask this comment to be restored, because it isn't obvious without the comment.

[robrap]

Even though there are a bunch of comments, I think this is close. Thanks. The tests don't seem overly dense and redundant any longer.

[robrap]

Is there supposed to be a count associated with this?

[ktyagiapphelix2u]

I added the missing count tracking

[robrap]

I guess we can just go with the final return in this method.

Suggested change

return null_config_count

[robrap]

Why is this something that needs to be mocked? This is not a case we can get to with regular test data?

[ktyagiapphelix2u]

Got it i have fixed that

[robrap]

It may be useful to add (or restore) a quick comment about why this is 2 and not 1.

[robrap]

Same note about the 2. You'll want a short comment explaining.

[robrap]

Assuming the following is accurate, you could add this test higher up in the file and then
you could note in comments where this goes from 1 to 2, that the setup data accounts for 1.

def test_run_checks_setup_test_data(self):
        """
        Test the --run-checks command against initial setup test data.
        """
        output = self._run_checks_command()

        # The setup data includes a saml provider with a missing config
        self.assertIn('Missing configs: 1', output)

UPDATE: This is optional. It could even check the warning. Or, you could leave this out if you appropriately comment the other 2s.

[ktyagiapphelix2u]

Done! Added test_run_checks_setup_test_data to validate the baseline setup data. Updated all test assertions with clarifying comments - the setUp contributes 1 to "Disabled configs" (not "Missing configs" as I initially thought). All tests now pass and clearly show how counts change from the baseline.

[robrap]

[inform] I did ask this comment to be restored, because it isn't obvious without the comment.