MethodsAndPractices / vsteam

PowerShell module for accessing Azure DevOps Services and Azure DevOps Server (formerly VSTS or TFS)
https://methodsandpractices.github.io/vsteam-docs/
MIT License
447 stars 155 forks source link

Get-VSTeamUserEntitlement only returns 96 results #388

Closed Rincey closed 2 years ago

Rincey commented 3 years ago

Steps to reproduce

$users = Get-VSTeamuser -SubjectTypes aad
$userentitlements = Get-VSTeamUserEntitlement

Expected behaviour

$users.count
359

$users | ? mailaddress -eq "<my email address>"
<returns information>

$userentitlements.count
359
$userentitlements | ? displayname -eq "<my first name>*"
<returns information>

Actual behaviour

$userentitlements.count
96
$userentitlements | ? displayname -eq "<my first name>*"
<returns nothing>

Environment data

OS

Server

Cannot get anything back for $VSTeamVersionTable However:

get-module vsteam -ListAvailable

    Directory: C:\Program Files\WindowsPowerShell\Modules

ModuleType Version    Name                                ExportedCommands
---------- -------    ----                                ----------------
Script     7.1.3      VSTeam                              {Add-VSTeam, Add-VSTeamAccessControlEntry, Add-VSTeamArea, Add-VSTeamAzureRMServiceEndpoint...}
> Get-VSTeamAPIVersion
Name                           Value
----                           -----
Packaging                      6.0-preview
Version                        VSTS
ServiceEndpoints               5.0-preview
HierarchyQuery                 5.1-preview
Graph                          6.0-preview
Pipelines                      5.1-preview
TaskGroups                     6.0-preview
ExtensionsManagement           6.0-preview
Git                            5.1
VariableGroups                 5.1-preview.1
Core                           5.1
Release                        5.1
DistributedTaskReleased        5.1
MemberEntitlementManagement    6.0-preview
Tfvc                           5.1
Processes                      6.0-preview
Policy                         5.1
Build                          5.1
DistributedTask                6.0-preview
> $PSVersionTable
Name                           Value
----                           -----
PSVersion                      5.1.19041.610
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.19041.610
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
mnieto commented 2 years ago

I my case, it returns always the same first 77 users.

PS> (Get-VSTeamUserEntitlement).Count 77 PS> (Get-VSTeamUserEntitlement -Top 1).Count 77 PS> (Get-VSTeamUserEntitlement -Top 1000).Count 77

I have 1539 users VSTeam version 7.4.0 Checked in both Desktop and Core powershell versions

Get-VSTeamAPIVersion

Billing                     : 5.1-preview.1
Build                       : 5.1
Core                        : 5.1
DistributedTask             : 6.0-preview
DistributedTaskReleased     : 5.1
ExtensionsManagement        : 6.0-preview
Git                         : 5.1
Graph                       : 6.0-preview
HierarchyQuery              : 5.1-preview
MemberEntitlementManagement : 6.0-preview
Packaging                   : 6.0-preview
Pipelines                   : 5.1-preview
Policy                      : 5.1
Processes                   : 6.0-preview
Release                     : 5.1
ServiceEndpoints            : 5.0-preview
TaskGroups                  : 6.0-preview
Tfvc                        : 5.1
VariableGroups              : 5.1-preview.1
Version                     : AzD

As side note: calling from Postman directly also returns the same 77 objects https://vsaex.dev.azure.com/organization/_apis/userentitlements?api-version=6.0

SebastianSchuetze commented 2 years ago

I think the wrong API version being used is the problem. Either the documentation or the API is wrong.

mnieto commented 2 years ago

Thanks, changing Set-VSTeamAPIVersion -Service MemberEntitlementManagement -Version 5.1-preview.2 it did the work

I think this is because the REST API changed some time from v5.1 to v6.0

When VSTeam module is installed, is pointing to the last MemberEntitlementManagement service version. So, it would be nice if Get-VSTeamUserEntitlement is changed to the last version behavior or preset the MemberEntitlementManagement service to the 5.1 version

SebastianSchuetze commented 2 years ago

Thanks for verification. How would you suggest to update it without breaking earlier scripts? Any idea?

mnieto commented 2 years ago

I think it's complicated to change the internal implementation keeping the current parameters For the -top parameter: Use the continuation token to call the API up to get the -top n rows, return that n rows and keep the excding rows foreseing a next call For the -skip parameter, Keep the status (the continuation token) between calls I can't figure any update without saving some internal status between calls. And that seems me an awfull solution.

Other option

Use different parameter set to separate both versions:

and warn some how in the documentation that to use the top & skip version it's needed to keep the MemberEntitlementManagement service lower or equal to 5.1 version calling to Set-VSTeamAPIVersion -Service MemberEntitlementManagement -Version 5.1-preview.2

SebastianSchuetze commented 2 years ago

Good idea thanks!

I will think about it. Otherwise we have to up the mayor version.

mnieto commented 2 years ago

I would like to propose this new syntax for the command. If you agree I can try to go to a PR

Get-VSTeamUserEntitlement [-Top <int>] [-Skip <int>] [-Select <string[]>] [<CommonParameters>]
Get-VSTeamUserEntitlement [-Id <string[]>] [<CommonParameters>]
Get-VSTeamUserEntitlement [-Select <string[]>] [-ContinuationToken <string>] [-Filter <string>] [<CommonParameters>]
Get-VSTeamUserEntitlement [-Select <string[]>] [-ContinuationToken <string>] [-License <string>] [-LicenseStatus <string>] [-UserType <string>] [-Name <string>] [<CommonParameters>]

The first 2 entries are the current ones.

Now, the new parameter sets:

The param list could be somthing like this:

   param (
      [Parameter(ParameterSetName = 'List')]
      [int] $Top = 100,

      [Parameter(ParameterSetName = 'List')]
      [int] $Skip = 0,

      [Parameter(ParameterSetName = 'List')]
      [Parameter(ParameterSetName = 'ContinuationTokenFilter')]
      [Parameter(ParameterSetName = 'ContinuationTokenParams')]
      [ValidateSet('Projects', 'Extensions', 'Grouprules')]
      [string[]] $Select,

      [Parameter(ParameterSetName = 'ByID')]
      [Alias('UserId')]
      [string[]] $Id,

      [Parameter(ParameterSetName = 'ContinuationTokenFilter')]
      [Parameter(ParameterSetName = 'ContinuationTokenParams')]
      [string] $ContinuationToken = $null,

      [Parameter(ParameterSetName = 'ContinuationTokenFilter')]
      [string] $Filter,

      [Parameter(ParameterSetName = 'ContinuationTokenParams')]
      [ValidateSet('List', 'Valid', 'License', 'Types')]
      [string] $License,

      [Parameter(ParameterSetName = 'ContinuationTokenParams')]
      [ValidateSet('Disabled', 'Enabled')]
      [string] $LicenseStatus,

      [Parameter(ParameterSetName = 'ContinuationTokenParams')]
      [ValidateSet('Guest', 'Member')]
      [string] $UserType,

      [Parameter(ParameterSetName = 'ContinuationTokenParams')]
      [Alias('UserName')]
      [Alias('Mail')]
      [string] $Name
   )

Official documentation for reference: https://docs.microsoft.com/en-us/rest/api/azure/devops/memberentitlementmanagement/user-entitlements/search-user-entitlements?view=azure-devops-rest-6.0

@SebastianSchuetze, what do you think?

SebastianSchuetze commented 2 years ago

@mnieto sorry for the silence. You can go with it and even change the parameters of the cmdlet if necessary. We will drop support of older on-premise versions anyways with version 8.0.

Ant regards to the change of the parameters. I would suggest that you make an extended explanation for the breaking change in the changelog.

As you suggested please check for the cmdlet itself what version we use. And throw an exception to change the API. Good point!

Hope you are still open for a PR.

SebastianSchuetze commented 2 years ago

Btw. regarding the continuation token. There will be multiple API calls, where there are continuation tokens. If you see you can create an internal function that can be reused regarding those tokens then I would appreciate it.

Maybe future implementations could be easier.

mnieto commented 2 years ago

@SebastianSchuetze , working on it. I have some doubts: How do you think is the best way to return the ContinuationToken?

SebastianSchuetze commented 2 years ago

Hi @mnieto I would go for the third option for now. I think the continuation token complexity should be hidden as much as possible. We can expose this function later if really needed. However, I would suggest an optional parameter with something like "-MaxPages" (int) or similar which limits how much requests we do. So the user can decide if they want only 5 pages or all.

Default should be a setting that returns all pages. Maybe 0 could mean "all" and is the default.

SebastianSchuetze commented 2 years ago

@mnieto could I ask how far you have come? If not then no problem. But then I would take an approach myself. Would like to have this added soon.

mnieto commented 2 years ago

I created tests and documentation. Now trying how to figure out a generic method to manage the continuationToken. Still working most of the changes on my local fork, not PR created yet. I can create it, so you can review it, but it's a work in progress. Not sure how to proceed (I think it is my first PR):

Any case, I will try to advance this week

SebastianSchuetze commented 2 years ago

You can already make a draft PR. Then you can see already if the build is accepted. I have also open PRs already. And I could help with tips or even commit to the PR branch if needed.

SebastianSchuetze commented 2 years ago

You seem to have progress with your PR @mnieto do you need something from me to help?

mnieto commented 2 years ago

Hi @SebastianSchuetze I'm investigating how to extract the continuationToken management block to a generic function. https://github.com/MethodsAndPractices/vsteam/blob/79d9801102d47e837f4992b664f52f0ba0f149a3/Source/Public/Get-VSTeamUserEntitlement.ps1#L110-129

I see some problems here:

mnieto commented 2 years ago

Finaly I resolved as below. If it is ok for you, I'm ready to finish the PR

Method: In a future, it may be necessary add the same parameters that _callAPI currently has, but at this moment I prefer to keep it simple

function _callAPIContinuationToken{
   [CmdletBinding()]
   param(
      [string]$Url,
      [string]$ContinuationTokenName,
      [string]$PropertyName,
      [int]$MaxPages
   )

   if ($MaxPages -le 0){
      $MaxPages = [int32]::MaxValue
   }
   if ([string]::IsNullOrEmpty($ContinuationTokenName)) {
     $ContinuationTokenName = "continuationToken"
   }
   $i = 0
   $obj = @()
   $apiParameters = $url
   do {
         $resp = _callAPI -url $apiParameters
         $continuationToken = $resp."$ContinuationTokenName"
         $i++
         Write-Verbose "page $i"
         $obj += $resp."$PropertyName"
         if (-not [String]::IsNullOrEmpty($continuationToken)) {
            $continuationToken = [uri]::EscapeDataString($continuationToken)
            $apiParameters = "${url}&continuationToken=$continuationToken"
         }
   } while (-not [String]::IsNullOrEmpty($continuationToken) -and $i -lt $MaxPages)

   return $obj
}

Call:

    $items = _callAPIContinuationToken -Url $listurl -PropertyName "members"
    foreach ($item in $items) {
       $objs += [vsteam_lib.UserEntitlement]::new($item)
    }
SebastianSchuetze commented 2 years ago

I will have a look as soon as I have some time. Hope it takes not too long. Could you add comments to code lines where somebody might not expect what is happening with the code?

If you think it is obvious then just wait for my questions. 😉

Thanks already for the great help!!

mnieto commented 2 years ago

@SebastianSchuetze, any chance to review my last comments on PR #459 ?

SebastianSchuetze commented 2 years ago

I need to find time to get I to it again regarding the one comment I made with the parameters. I try to check it this weekend.

SebastianSchuetze commented 2 years ago

@mnieto unfortunately I discovered a problem. The integration test have an error:

https://github.com/MethodsAndPractices/vsteam/runs/8176556019?check_suite_focus=true#step:7:121

RuntimeException: EntitlementManagemen version must be equal or lower than 5.1 for this call, current value 6.0-preview

by calling the following function

Get-VSTeamUserEntitlement -Id $id

This means that your cmdlet changes actually change the required parameters for existing scripts. Either I didn't get it and the parameter set 'ByID' must also be allowed for apiVersion 5.1 and above or I have to change the mayor version. I would rather have the first. I am researching currently. But maybe you can help me explaining how to solve it.

mnieto commented 2 years ago

@SebastianSchuetze, Understood. The main problem is that the new API in v6.0 doesn't support filtering by ID. See $filter parameter. The workaround I see is: check if ParameterSetName -eq 'ById' set internally $apiVersion to 5.1 independently of the version configured in the Set-VSTeamAPIVersion Let me try along this week

mnieto commented 2 years ago

@SebastianSchuetze I created PR #482 to fix the back-compability issue not sure how to update the changelog in this case

SebastianSchuetze commented 2 years ago

I close this now since it is working again.