okta / okta-sdk-dotnet

A .NET SDK for interacting with the Okta management API, enabling server-side code to manage Okta users, groups, applications, and more.
Other
160 stars 100 forks source link

UserSchemaAttributes minLength and maxLength are non-nullable and cause array schema attribute creation to fail. #702

Closed rcollette closed 5 months ago

rcollette commented 8 months ago

Describe the bug?

SMH. At what point does Okta realize that we are paying customers, and it is just embarrassing how many bugs there are that are not being fixed in this SDK?

In the UserSchemaAttribute class, the properties minLength and maxLength are non-nullable. When creating an array type attribute, minLength and maxLength properties are being sent with a value of 0. This causes ISchemaApiAsync.UpdateApplicationUserProfileAsync() to fail with the error message Api validation failed: The type: array is not a string type and supported with minLength and maxLength.

What is expected to happen?

minLength and maxLength properties should not be sent (null)

What is the actual behavior?

minLength and maxLength properties are sent with a zero value.

Reproduction Steps?

Get an existing UserSchema and add an array attribute to it

Dictionary<string, UserSchemaAttribute> customAttributes = userSchema.Definitions.Custom.Properties ??
                                                                   throw new InvalidOperationException(
                                                                       "Custom attributes are null.");
 var userSchemaAttribute = new UserSchemaAttribute
        {
            Type = UserSchemaAttributeType.Array,
            Title = "someName",
            Required = false,
            Scope = UserSchemaAttributeScope.NONE,
            ExternalName = "someExternalName",
            Items = new UserSchemaAttributeItems { Type = "string" }
        };
customAttributes.Add(attributeName, userSchemaAttribute);
schemaApiAsync.UpdateApplicationUserProfileAsync(idpId,userSchema)

Additional Information?

No response

.NET Version

8.0.200

SDK Version

8.0.200

OS version

Darwin MacBook-Pro-3.local 23.2.0 Darwin Kernel Version 23.2.0: Wed Nov 15 21:53:18 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T6000 arm64

laura-rodriguez commented 8 months ago

Thank you so much for taking the time to report this issue and sharing your concerns with us. We take your feedback seriously. We have been surfacing these concerns to the team and will be working on improvements to the OpenAPI specification and SDKs. I'll make sure to give visibility to this, and other issues so we can prioritize them as soon as possible.

Internal ref: OKTA-709693 and OKTA-719172

rcollette commented 7 months ago

I am working around this issue currently by defining a UserAttribute converter.

SDK configuration

services.AddSingleton<ISchemaApiAsync>(
    serviceProvider =>
    {
        IOptions<Configuration> options = serviceProvider.GetService<IOptions<Configuration>>() ??
                                          throw new ConfigurationErrorsException(
                                              "Unable to get the okta configuration");
        Configuration configuration = options.Value;
        Okta.Sdk.Client.Configuration.Validate(configuration);
        IOAuthTokenProvider? oAuthTokenProvider = NullOAuthTokenProvider.Instance;
        if (Okta.Sdk.Client.Configuration.IsPrivateKeyMode(configuration))
        {
            oAuthTokenProvider ??= new DefaultOAuthTokenProvider(configuration);
        }

        ApiClient apiClient = new(configuration.OktaDomain, oAuthTokenProvider);
        apiClient.SerializerSettings.Converters.Add(new UserSchemaAttributeConverter());
        SchemaApi oktaClient = new(apiClient, options.Value);
        return oktaClient;
    });

Converter

using System;
using System.Collections.Generic;
using System.Linq;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Okta.Sdk.Model;

public class UserSchemaAttributeConverter : JsonConverter
{
    public override bool CanConvert(Type objectType)
    {
        return objectType == typeof(UserSchemaAttribute);
    }

    private static JsonSerializerSettings CloneSerializerSettings(JsonSerializer serializer)
    {
        return new JsonSerializerSettings
        {
            Culture = serializer.Culture,
            DateFormatHandling = serializer.DateFormatHandling,
            DateTimeZoneHandling = serializer.DateTimeZoneHandling,
            DefaultValueHandling = serializer.DefaultValueHandling,
            Formatting = serializer.Formatting,
            NullValueHandling = serializer.NullValueHandling,
            ObjectCreationHandling = serializer.ObjectCreationHandling,
            ReferenceLoopHandling = serializer.ReferenceLoopHandling,
            StringEscapeHandling = serializer.StringEscapeHandling,
            TypeNameHandling = serializer.TypeNameHandling,
            MetadataPropertyHandling = serializer.MetadataPropertyHandling,
            Converters = [.. serializer.Converters]
        };
    }

    public override object? ReadJson(
        JsonReader reader,
        Type objectType,
        object? existingValue,
        JsonSerializer serializer)
    {
        JsonSerializerSettings settings = CloneSerializerSettings(serializer);
        settings.Converters.Remove(this); // Remove this converter to avoid recursion

        JsonSerializer localSerializer = JsonSerializer.Create(settings);
        return localSerializer.Deserialize(reader, objectType);
    }

    public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer)
    {
        if (value == null)
        {
            return;
        }

        UserSchemaAttribute userSchemaAttribute = (UserSchemaAttribute)value;
        JsonSerializerSettings settings = CloneSerializerSettings(serializer);
        settings.Converters.Remove(this); // Remove this converter to avoid recursion
        JsonSerializer localSerializer = JsonSerializer.Create(settings);
        JObject userSchemaAttributeJObject = JObject.FromObject(userSchemaAttribute, localSerializer);
        if (userSchemaAttribute.Type == UserSchemaAttributeType.Array)
        {
            // Due to bug https://github.com/okta/okta-sdk-dotnet/issues/702, maxLength and minLength are being sent
            // as values of zero when they should be null/not-present for arrays.
            userSchemaAttributeJObject.Remove("maxLength");
            userSchemaAttributeJObject.Remove("minLength");
            JObject? itemsJObject = (JObject?)userSchemaAttributeJObject["items"];
            if (itemsJObject != null)
            {
                // Due to bug https://github.com/okta/okta-sdk-dotnet/issues/712, null values are being sent for properties and need to be removed.
                RemoveNullValues(itemsJObject);
            }
        }

        // Due to bug https://github.com/okta/okta-sdk-dotnet/issues/712, null values are being sent for properties and need to be removed.
        RemoveNullValues(userSchemaAttributeJObject);
        userSchemaAttributeJObject.WriteTo(writer);
    }

    private static void RemoveNullValues(JObject jObject)
    {
        List<string> propertiesToRemove = jObject.Properties()
            .Where(prop => prop.Value.Type == JTokenType.Null)
            .Select(prop => prop.Name)
            .ToList();
        foreach (string property in propertiesToRemove)
        {
            jObject.Remove(property);
        }
    }
}
bryanapellanes-okta commented 6 months ago

@rcollette I appreciate your patience while we address this issue. I've submitted a pull request with the changes necessary to address this bug. Please feel free to review and comment while we also review within the team: https://github.com/okta/okta-sdk-dotnet/pull/713

bryanapellanes-okta commented 5 months ago

This is corrected in v8