DHIS2: Schema should not require username/password when using a PAT

[PiusKariuki]

Summary

The configuration schema requires username and password even when authenticating with a PAT, forcing users to provide credentials that are silently ignored.

The schema currently has:

"required": ["hostUrl", "password", "username"]

And configureAuth uses a priority chain — PAT wins over password:

if ('pat' in auth) {
  Object.assign(headers, { Authorization: `ApiToken ${auth.pat}` });
} else if ('password' in auth) {
  Object.assign(headers, makeBasicAuthHeader(auth.username, auth.password));
}

The password branch doesn't check that username is also present, so CLI users (where the schema isn't enforced) can end up with silent auth failures.

Fixes #1547

Details

The schema currently has:

"required": ["hostUrl", "password", "username"]

And configureAuth uses a priority chain — PAT wins over password:

if ('pat' in auth) {
  Object.assign(headers, { Authorization: `ApiToken ${auth.pat}` });
} else if ('password' in auth) {
  Object.assign(headers, makeBasicAuthHeader(auth.username, auth.password));
}

The password branch doesn't check that username is also present, so CLI users (where the schema isn't enforced) can end up with silent auth failures.

Expected behaviour

1. Schema should express that credentials are one-of: either pat, or both username + password — not all three required. A oneOf/anyOf structure would work.
2. Runtime: when falling back to Basic auth (no PAT), configureAuth should validate that both username and password are present and non-empty, and throw a descriptive error if either is missing.

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to
know!):

• I have used Claude Code
• I have used another model
• I have not used AI

You can read more details in our
Responsible AI Policy

Review Checklist

Before merging, the reviewer should check the following items:

• Does the PR do what it claims to do?
• If this is a new adaptor, added the adaptor on marketing website ?
• If this PR includes breaking changes, do we need to update any jobs in
  production? Is it safe to release?
• Are there any unit tests?
• Is there a changeset associated with this PR? Should there be? Note that
  dev only changes don't need a changeset.
• Have you ticked a box under AI Usage?

[mtuchi]

I think on the unit test for configureAuth could be summarised into

it('should throw if no valid credentials are provided', () => {
  expect(() => util.configureAuth({})).to.throw('Invalid authorization...');
});

it('should throw if username is provided without a password', () => {
  expect(() => util.configureAuth({ username: 'admin', password: '' })).to.throw('Invalid authorization...');
});

I think the description should be very specific and tidy @PiusKariuki

[mtuchi]

@PiusKariuki where i am getting at is for example this description is not accurate
"should throw if pat is empty" because it will only throw if pat is empty and username and password is not provided

[josephjclark]

Yeah these tests are a little peculiar.

They look thorough at first glance but:

• in the test should throw if pat is empty, user name and password are all empty
• you're testing "empty" but not null or undefined. Why is "empty" so significant?

They're not very tight, accurate, controlled tests. And they're actually quite misleading: should throw if pat is empty is simply not a reflection of the spec. I agree with Mtuchi's approach

[josephjclark]

You might also test for example that pat is preferred to username and password - that feels like an important thing to a) document in a test and b) preserve in a regression test

[mtuchi]

• "should prioritize pat over username/password when both are present"
• "should set Authorization header with ApiToken when pat is provided"
• "should set Basic Auth header when username and password are provided (no pat)"

[mtuchi]

@PiusKariuki can you update the pr checklist notes and title, also since you updated the util we should have a changeset

[PiusKariuki]

@mtuchi You would like less tests? I think that would reduce our coverage in testing what was written in the spec. See link See below for things we would miss out on:

1. Not testing the scenario where an empty string is passed in place of a PAT.
2. Not testing the scenario where the configureAuth function would not check if a  ​username is present.

I have added a changeset file as requested and the PR checklist notes and title

[PiusKariuki]

@josephjclark @mtuchi Got it. Thank you both for clarifying those issues on the tests. Key changes I have made:

1. Dropped the misleading should throw if pat is empty test.
2. Updated descriptions to accurately reflect what is being tested.
3. Added the PAT priority test.