feat: Fetch service account OIDC data

[bec-callow-oct]

Background

[SC-136951]
For the terraform changes in this PR OctopusDeploy/terraform-provider-octopusdeploy#176, the client needs to return the service account external ID so that it can be used as an output by terraform. The Octopus API returns this information via the /api/serviceaccounts/Users-83/oidcidentities/v1?skip=0&take=1 endpoint which the GetOIDCIdentities function was attempting to call but there were a few problems:

1. the function only attempted to return the OIDC Identities as a list of resources and did not include fields such as External ID from the body of the json.
2. the route template (api/serviceaccounts/{serviceAccountId}/oidcidentities/v1{?skip,take}) requires serviceAccountId but that was not being provided either as a method parameter or part of the query object.
3. The OIDC Identities array does not match the required format for mapping to the Resources type since we expect the array to be named "Items", along with a few other issues.

I've added the serviceAccountID to the query object so the original function can hit the correct endpoint now, however the response mapping is still broken.

Testing

Go snippet for testing:

	query := serviceaccounts.OIDCIdentityQuery{
		ServiceAccountId: serviceAccountID,
		Skip:             0,
		Take:             10,
	}

	// Test the original GetOIDCIdentities function which does not work
	fmt.Println("\n=== GetOIDCIdentities function ===")
	identities, err := serviceaccounts.GetOIDCIdentities(octopusClient, query)
	if err != nil {
		fmt.Printf("Error: %v\n", err)
	} else {
		fmt.Printf("   Items: %d, Total: %d\n", len(identities.Items), identities.TotalResults)
	}

	// Test the new GetServiceAccountOIDCData function
	fmt.Println("\n=== GetServiceAccountOIDCData function ===")
	oidcData, err := serviceaccounts.GetServiceAccountOIDCData(octopusClient, query)
	if err != nil {
		fmt.Printf("FAILED: %v\n", err)
	} else {
		fmt.Printf("   ExternalId: %s\n", oidcData.ExternalId)
		fmt.Printf("   OIDC Identities: %d\n", len(oidcData.OidcIdentities))
	}

Results

The GetOIDCIdentities fails to map the response correctly so returns no data. The ExternalId and OIDC Identities are successfully returned by the new function.

=== GetOIDCIdentities function ===
   Items: 0, Total: 0

=== GetServiceAccountOIDCData function ===
   ExternalId: 6b453c3a-0e28-4a69-8bf6-91f58c9ea0a3
   OIDC Identities: 1

[bec-callow-oct]

I don't think this function has ever worked. I'm considering removing it but would like another opinion on whether to fix or remove

[benPearce1]

Since this is a library being used by customers, we should try to fix it rather than delete. We aren't the only users of this library.
Log an issue if you can determine how it is broken.

[Rose-Northey]

I'm guessing you've tested these with terraform. But is there a reason its difficult to have tests in go as well? Is it because of the difficulty of creating a service account for it? Is a service account the account you have with an auth application? so you'd need to simmulate an account with an external application?

[bec-callow-oct]

Yes, I've tested manually, both using the client directly and via terraform. There are no existing tests for OIDC identities to work from and the setup looks fairly complex

[Rose-Northey]

Would adding this field break old usages of the query if they were wotking?

[bec-callow-oct]

If there were working usages of this query, adding ServiceAccountId would not break them since it would default to empty string, but this query was only used for the GetOIDCIdentities function which was calling api/serviceaccounts//oidcidentities/v1{?skip,take} when the service account ID was not provided (note the double forward slash) and that endpoint does not exist on Octopus server.

The correct endpoint is api/serviceaccounts/{serviceAccountId}/oidcidentities/v1{?skip,take} where service account ID, Skip and Take are all required

[bec-callow-oct]

Maybe if someone had used the query to build a custom request so that they could bypass the broken function, there could be a working use of this and I guess we can't exactly predict whether the changes could break custom code. I still think this is a worthwhile change, but could release it as a potentially breaking change

[Rose-Northey]

Yeah makes sense. Seems like the custom code situation would be super edge case

[Rose-Northey]

thanks for answering my questions