mattmcnabb / onelogin Goto Github PK

When filtering for events, if the value of the filter parameter is null, the command will happily accept this and go on as if no filter was applied. This example will return every event:

Get-OneLoginEvent -Filter @{user_id = $null}

The same thing would occur if I passed in a variable that did not exist, such as in the case of a typo:

$User = Get-OneLoginUser -Filter @{email = "[email protected]"}
Get-OneLoginEvent -Filter @{user_id = $Users.Id}

There should be some validation on the filters to ensure that null values are not passed in.

Filtering in the OneLogin API is case-sensitive, but the validation of filtering parameters is not. This can result in 400: bad request errors being returned to the user.

Example:

# this works
Get-OneLoginEvent -Filter @{user_id = '123456'}

# this returns a 40 error
Get-OneLoginEvent -Filter @{User_Id = '123456'}

The -SetAsDefault parameter of New-OneLoginToken does not work when the module is installed via the PSGallery. This is because the module name is determined by file path and when installed from the Gallery the parent folder path is the version number, not the name of the module. this is a dumb way to do it so looking for another way to get the module metadata from within the function.

sometimes a new property is added to an object in the API, and a PowerShell object will not be instantiated but will return an error. A method is needed to return user-friendly errors describing that problem and the list of properties returned from the API.

Needs

• a Onelogin.Exception class for easily building new errors
• exception message enum
• can this all be handled centrally in Invoke-OneLoginRestMethod, rather than in each function?

Currently I'm using custom cSharp classes to manage OneLogin objects. The custom objects returned by the REST api are simply cast to the custom types. This works in general but if type conversion fails for a property then no object is output. What's the solution to this?

Is the solution to use constructors instead of casts? If so, how should this be handled with a large number of possible properties?

Can I build these constructors into convertto-OneLoginObject?

The filtering experience is not very user friendly. How can this be improved?

Parameter filtering

• Leverage parameters to implement filtering and do all the dirty work inside the function body
• the advantage here is that all the filter parameters can be autocompleted
• parameter names would need to indicate that they are filter parameters

String filtering

• use a filtering approach like the AD cmdlets - "email -eq 'matt*'"
• this would require some serious parsing, but that parsing may be able to be centralized in a shared helper

hashtable filtering

• Continue using hashtables similar to Get-WinEvent
• This works well with multiple filter parameters
• possible values are hard to discover
• could write some logic in to better handle values passed in via enums

Instead of collecting a token and passing it to the functions, can the module share a script scope variable created in a connect-OneLogin function?

This seems to be required for Put and Post methods, and converting all is cleaner than supporting both hashtables and json strings.

Running:

Get-OneLoginEvent -Filter @{user_id = "123456"; directory_id = '57489'}

returns error:

Get-OneLoginEvent : Cannot validate argument on parameter 'Filter'. [user_id directory_id] is not a filterable property. Filterable properties are
[client_id, created_at, directory_id, event_type_id, resolution, user_id]. These properties are CASE-SENSITIVE!
At line:1 char:27
+ ... t-OneLoginEvent -Filter @{user_id = "123456"; directory_id = '57489'}
+                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidData: (:) [Get-OneLoginEvent], ParameterBindingValidationException
    + FullyQualifiedErrorId : ParameterArgumentValidationError,Get-OneLoginEvent

I should be able to use multiple filter properties.

None of the commands have any help documentation, and there are no about topics either.

the API docs detail which user properties can be used to query the endpoint, and lastname is conspicuously absent. It appears that lastname is a filterable property, so docs probably need updating. Will add this one property as a filter parameter and ask dev support whether there are other undocumented query parameters.

master branch automatically deploys
push development to dev branch - this is where regular testing will take place
only push to master when absolutely sure a new deployment is ready
how can this be managed/controlled?
feature branches are local
leverage native appveyor build, test and deploy

when -All is specified, all pages returned by the API are unfolded and this can result in long delays depending on the number of users returned. Can paging be handled in a more elegant way?

How is paging handled if filtering is used but more than 50 objects are returned? Will the paging loop handle this?

How can we determine the number of objects returned prior to requesting all the pages?

Full test suite
Invoke-OneloginRestMethod is of particular importance and might need to be broken down into two or three smaller functions:

• one for gets
• one for put/post/del
• one for pagination

No errors are returned, but nothing is output. I suspect this has something to do with type conversion on the OneLoginApp class. Maybe the provisioned property?

Can we pipe a user to Get-OneloginEvent? Where else would it make sense to pipe objects?

This seems like something an application might need, but not something an admin might need.

This just returns "OneLogin.User" as the target. Can you fix this?

This should not set default values for New-OneLoginToken or New-OneLoginRefreshToken

large data sets just keep returning the first page over and over again. I suspect there is a fault in the logic of Invoke-OneLoginRestMethod.

Currently this module does not work in PowerShell Core. when you attempt to run Connect-OneLogin, Invoke-restmethod returns an error that includes the value of the client secret. This needs to be fixed ASAP as a security flaw.

see issue title

In theory this could support versions 3 and 4 but needs better testing

Roles are retrieved based on the user object piped in - if the user object is saved in a variable then any new roles since the user was retrieved will not be included.

Repro:

• Step 1

$User = Get-OneloginUser -filter @{email = "matt@github*"}

• Step 2
  Mappings are reapplied and a new role is added.

• Step 3

$User | Get-OneLoginUserRole

The new role is not returned. This is because the roles are retrieved based on the role ids included in the user object which has been saved statically in the $user variable. Get-OneLoginUserRole should re-retrieve these role ids before returning the role objects.

This is extraneous and could stand DRYing up. This is a module level variable and so can be called upon right in the Invoke-OneLoginRestMethod function.

Currently the Onelogin api docs state that there are 6 possible values for user status:

0: Unactivated
1: Active
2: Suspended
3: Locked
4: Password expired
5: Awaiting password reset

However, in at least one case I've seen a user return a value of 8 for this. I've posed a question on stack overflow to try to find out whether this doc is out of date and what the additional values may be. In the meantime the OneLoginUser class will report "Unknown" for any values outside of the known range.

currently have 100% test coverage, but still not confident that pagination of API responses is fully tested. How can this be tested more accurately? Will it help to modularize the pagination into a separate function?

using a filter and the -since and -until parameters doesn't seem to work right - filtering to a user seems to return events for all users.

All object datetime properties should display local times, and any calls to the API should convert local time to UTC for accuracy. Can [DateTimeOffset] fulfill this need?

Originally classes were dot-sourced in the psm1 file and thus were only available in module scope. This worked fine for custom object output and parameter type declarations. However, this did not allow for using the [outputtype()] attribute, and also did not allow investigation of enum values at the command line. To resolve this I chose to evaluate the class files via the ScriptsToProcess property of the module manifest. This brings the types into the caller scope while still allowing parameter type constraints.

When I create a new function that accepts an existing OneLogin class as parameter input, I have to launch a new PowerShell console and import the module there. If I attempt to re-import the module, even with the force switch, I get the error:

New-OneLoginRefreshToken : Cannot process argument transformation on parameter 'Token'. Cannot convert the "OneLoginToken" value of type "OneLoginToken" to type "OneLoginToken". At line:1 char:33

New-OneLoginRefreshToken -Token $token

CategoryInfo : InvalidData: (:) [New-OneLoginRefreshToken], ParameterBindingArgumentTransformationException
FullyQualifiedErrorId : ParameterArgumentTransformationError,New-OneLoginRefreshToken

Running this:

Import-Module .\onelogin.psd1 -Force
$OLCred = Import-Clixml .\cred_user.xml
$Token = New-OneLoginToken -Credential $OLCred

Works just fine in the console, but not if run in a script:

Cannot convert the "OneLoginToken" value of type "System.String" to type "System.Type".
At C:\repos\OneLogin\helpers\ConvertTo-OneLoginObject.ps1:19 char:13
+             $InputObject -as $TypeName
+             ~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidArgument: (:) [], RuntimeException
    + FullyQualifiedErrorId : InvalidCastFromStringToType

However, the same script above works if dot-sourced. Clearly there is something I'm missing about script scope here, but I'm not sure what it is.

The API has a rate limit endpoint for determining how many calls have been made against the api with the current api credential. More information here:
https://developers.onelogin.com/api-docs/1/oauth20-tokens/get-rate-limit

The module should have a function that returns this data to the console.

This should return the text status rather than the code

From what I can tell, the OneLogin API Role endpoint is currently returning pagination data for even if the total count of roles is less than 50. This results in Invoke-OneLoginRestMethod making multiple paging calls and ending up with duplicates.

Described here.

This is the updated user class. Preferred_locale_code and manager_user_id
When invoking-restmethod the typecast failed. Might be worth explicitly casting properties via overloaded constructor to prevent future API changes.

public class User
{
    public DateTimeOffset? activated_at { get; set; }
    public string comment { get; set; }
    public string company { get; set; }
    public DateTimeOffset? created_at { get; set; }
    public object[] custom_attributes { get; set; }
    public string department { get; set; }
    public string directory_id {get; set;}
    public string distinguished_name {get; set;}
    public string email {get; set;}
    public string external_id {get; set;}
    public string firstname {get; set;}
    public string group_id {get; set;}
    public string id {get; set;}
    public string invalid_login_attempts {get; set;}
    public DateTimeOffset? invitation_sent_at {get; set;}
    public string lastname {get; set;}
    public DateTimeOffset? last_login {get; set;}
    public string locale_code {get; set;}
	public string preferred_locale_code {get; set;}
    public DateTimeOffset? locked_until {get; set;}
    public string manager_ad_id {get; set;}
	public string manager_user_id {get; set;}
    public string member_of {get; set;}
    public string[] notes {get; set;}
    public string openid_name {get; set;}
    public DateTimeOffset? password_changed_at {get; set;}
    public string phone {get; set;}
    public string[] role_id {get; set;}
    public string samaccountname {get; set;}
    public string state {get; set;}
    public string status {get; set;}
    public string status_value
    {
        get
        {
            try
            {
                return Enum.GetName(typeof(UserStatus), Int32.Parse(this.status));
            }
            catch
            {
                return "Unknown";
            }
        }
    }
    public string title {get; set;}
    public string trusted_idp_id {get; set;}
    public DateTimeOffset? updated_at {get; set;}
    public string username {get; set;}
    public string userprincipalname {get; set;}

    public override string ToString() { return this.id; }
}

right now the US region is hard-coded in the module. This should be configurable. Not sure how to achieve this yet - user config file? function that sets a variable or saves data in appdata? Maybe this could just be added to New-OneLoginToken along with the credential?

Could this be a more efficient method of returning available apps for a user?

https://developers.onelogin.com/api-docs/1/embed-apps/get-apps-to-embed-for-a-user

Hi,

Is it possible to have the 'status' property added to set-oneloginuser?

https://developers.onelogin.com/api-docs/1/users/update-user

I have modified the Set-oneloginuser.ps1 function to include the below:
[int]
$status

This is after the userPrincipalName line.

https://developers.onelogin.com/api-docs/1/users/set-password-using-sha-256

Right now .NET objects are instantiated via casting from the custom objects outputted by Invoke-RestMethod. This is flimsy because if the api begins returning a new property for an object, then the cast will fail silently and no error will be returned.

Constructors could be used to build the objects but these could be rather large as the objects like events have a large number of properties. I'm not sure if there is a shortcut way to construct these objects except using New-Object or a .NET constructor. Will need test to see which one is more reliable.

Some thoughts:

If New-Object is used and we can pass the PSCustomObject directly to the -Properties parameter, then we'll still likely get an error if a property exists that is not defined in the class. At least we'll get an error, though.

If we use constructors, we can control exactly which custom object properties will be passed in to create the object, and objects should always be instantiated. One possible drawback to this is that we'll never be alerted to the fact that new properties exist on the API objects. This shouldn't be a problem if the API docs are kept up to date.

This should build into a monolithic module in accordance with best practice.

OneLogin custom objects' date properties should be output as a datetime object, rather than a string. Also, this should convert UTC to local time and vice-versa.

enum names should be singular unless they are bit fields
write some help on using the enums if they are to be used for hashtable filtering

The API now has and event types endpoint

I believe this is a new addition, but can't verify. There could be a command for this to get an idea of what event types you want to look for. This might be better than using the filter hashtable.

Another idea is to use this to perform argument completion for an -EventType parameter.

if a user or event object has ID properties which are of type [int], and the property is null, 0 is returned. This should return null. One solution might be to simply return a string.

PSDeploy?
Appveyor?
TeamCity?
VSTS?
PSake?

Should support testing against multiple PowerShell versions