Add seperate Respone Schema for 2FA

[0xkubectl]

Fixes #241 - Based on the work of @C0D3-M4513R from this PR

Adds a new Response schema that is able to encapsulate the 2FA challange/request. This is done via the oneOf operator on the CurrentUserLoginResponse. Here are the docs for oneOf: https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/

I have tested this with the Rust Generator and have found no issues. I cannot say how other languages' generator would fare with this fix. If someone finds the time to test this on other languages, please go ahead :3

Tested languages

• [x] python @ 7.7.0
• [x] rust @ 7.7.0
• [x] typescript @ 7.7.0
• [ ] csharp @ ?
• [x] java @ 7.7.0
• [ ] dart @ ?

[C0D3-M4513R]

from #241:

You can specify that a code returns either, e.g. <User | ReqMFAAuth>, but almost no generator, and not all languages, support that. So due to this API design it is something we will have to fix in each SDK instead.

[0xkubectl]

Seen that, but i figured that the latest Rust generator is able to do this, it might be worth a try to bump the other languages aswell and see if that functionality exists.

[C0D3-M4513R]

Rust is a really, really powerful language when it comes to serialization and type constructs. A lot of other languages can't represent many things as neatly as rust can.

[0xkubectl]

Ok, ill just try the python one and see if it works :3

[C0D3-M4513R]

Also it would be interesting, if rust supports this without my manual patch or not. And of course other languages.

[0xkubectl]

I checked TS, Python and Rust, all of them can deal with the oneOf operator. Caveat: I bumped everyones openapi-generator version to 7.7.0

This indicates to me that there is at least baseline support for this.

[C0D3-M4513R]

List of generators that are documented to not work with oneOf: java: https://openapi-generator.tech/docs/generators/java/#schema-support-feature dart: https://openapi-generator.tech/docs/generators/dart#schema-support-feature javascript: https://openapi-generator.tech/docs/generators/javascript#schema-support-feature csharp: https://openapi-generator.tech/docs/generators/csharp#schema-support-feature

[0xkubectl]

Not sure what is up with the docs, but java for example generates this code, which looks pretty functional to me:

@Override
public GetCurrentUser200Response read(JsonReader in) throws IOException {
    Object deserialized = null;
    JsonElement jsonElement = elementAdapter.read(in);

    int match = 0;
    ArrayList<String> errorMessages = new ArrayList<>();
    TypeAdapter actualAdapter = elementAdapter;

    // deserialize CurrentUser
    try {
        // validate the JSON object to see if any exception is thrown
        CurrentUser.validateJsonElement(jsonElement);
        actualAdapter = adapterCurrentUser;
        match++;
        log.log(Level.FINER, "Input data matches schema 'CurrentUser'");
    } catch (Exception e) {
        // deserialization failed, continue
        errorMessages.add(String.format("Deserialization for CurrentUser failed with `%s`.", e.getMessage()));
        log.log(Level.FINER, "Input data does not match schema 'CurrentUser'", e);
    }
    // deserialize RequiresTwoFactorAuth
    try {
        // validate the JSON object to see if any exception is thrown
        RequiresTwoFactorAuth.validateJsonElement(jsonElement);
        actualAdapter = adapterRequiresTwoFactorAuth;
        match++;
        log.log(Level.FINER, "Input data matches schema 'RequiresTwoFactorAuth'");
    } catch (Exception e) {
        // deserialization failed, continue
        errorMessages.add(String.format("Deserialization for RequiresTwoFactorAuth failed with `%s`.", e.getMessage()));
        log.log(Level.FINER, "Input data does not match schema 'RequiresTwoFactorAuth'", e);
    }

    if (match == 1) {
        GetCurrentUser200Response ret = new GetCurrentUser200Response();
        ret.setActualInstance(actualAdapter.fromJsonTree(jsonElement));
        return ret;
    }

    throw new IOException(String.format("Failed deserialization for GetCurrentUser200Response: %d classes match result, expected 1. Detailed failure message for oneOf schemas: %s. JSON: %s", match, errorMessages, jsonElement.toString()));
    }
}.nullSafe();

Also the javascript library is generated with the typescript generator which seems also seems to say it does not support it, but generates these types just fine:

export type GetCurrentUser200Response = CurrentUser | RequiresTwoFactorAuth;

To be fair idk what to make of this.

[ariesclark]

We rely on the following generators:

• typescript-axios
• rust
• csharp-netcore
• python-legacy
• dart-dio
• java

Out of these, according to documentation, Only rust & dart-dio support oneOf. I see you've done testing above, does the actual support differ from the documented support?

[0xkubectl]

I found i quite strange that generators that list not having that feature, still create files that resemble to work. Looking into this i found the oneof template files for:

• java: https://github.com/OpenAPITools/openapi-generator/blob/e40d3228aa2256bb7a85fb6e26df9838f06ad79c/modules/openapi-generator/src/main/resources/Java/libraries/native/oneof_model.mustache#L108
• python: https://github.com/OpenAPITools/openapi-generator/blob/e40d3228aa2256bb7a85fb6e26df9838f06ad79c/modules/openapi-generator/src/main/resources/python/model_oneof.mustache
• csharp: https://github.com/OpenAPITools/openapi-generator/blob/e40d3228aa2256bb7a85fb6e26df9838f06ad79c/modules/openapi-generator/src/main/resources/csharp/modelOneOf.mustache
• typescript: https://github.com/OpenAPITools/openapi-generator/blob/e40d3228aa2256bb7a85fb6e26df9838f06ad79c/modules/openapi-generator/src/main/resources/typescript-axios/modelOneOf.mustache

My best guess is that oneOf can be used not just in schemas/models but also somewhere else and that you only claim full support when all those other usages are also working. Still emphazising that this is on 7.7.0 a major version above what is currently being used by everything but the rust language. I would propose we bump those versions and see if stuff breaks, and then come back to this PR.

[Rexios80]

oneOf definitely caused issues with the dart generator in the past although maybe we weren't using dart-dio then? I can try generating this next week.

[Rexios80]

This is what got generated:

GetCurrentUser200Response({
  required this.acceptedTOSVersion,
  this.acceptedPrivacyVersion,
  this.accountDeletionDate,
  this.accountDeletionLog,
  this.activeFriends,
  required this.allowAvatarCopying,
  this.badges,
  required this.bio,
  required this.bioLinks,
  required this.currentAvatar,
  required this.currentAvatarAssetUrl,
  required this.currentAvatarImageUrl,
  required this.currentAvatarThumbnailImageUrl,
  required this.currentAvatarTags,
  required this.dateJoined,
  required this.developerType,
  required this.displayName,
  required this.emailVerified,
  this.fallbackAvatar,
  required this.friendGroupNames,
  required this.friendKey,
  required this.friends,
  required this.hasBirthday,
  this.hideContentFilterSettings,
  this.userLanguage,
  this.userLanguageCode,
  required this.hasEmail,
  required this.hasLoggedInFromClient,
  required this.hasPendingEmail,
  required this.homeLocation,
  required this.id,
  this.isBoopingEnabled = true,
  this.isFriend = false,
  this.lastActivity,
  required this.lastLogin,
  required this.lastMobile,
  required this.lastPlatform,
  required this.obfuscatedEmail,
  required this.obfuscatedPendingEmail,
  required this.oculusId,
  this.googleId,
  this.googleDetails,
  this.picoId,
  this.viveId,
  this.offlineFriends,
  this.onlineFriends,
  required this.pastDisplayNames,
  this.presence,
  required this.profilePicOverride,
  required this.profilePicOverrideThumbnail,
  required this.pronouns,
  required this.state,
  required this.status,
  required this.statusDescription,
  required this.statusFirstTime,
  required this.statusHistory,
  required this.steamDetails,
  required this.steamId,
  required this.tags,
  required this.twoFactorAuthEnabled,
  this.twoFactorAuthEnabledDate,
  required this.unsubscribe,
  this.updatedAt,
  required this.userIcon,
  this.username,
  required this.requiresTwoFactorAuth,
});

It looks like it just combined the fields of both possible responses which definitely will not work

[ariesclark]

dart-dio does claim support for oneOf though, are you sure you're using the latest version of the generator?

[Rexios80]

Yes my generate script updates the generator and I double checked it's using 7.7.0

[0xkubectl]

After long debugging I found the issue - the serilization library is not compatible with oneOf. Commenting out serializationLibrary: json_serializable generated:

//
// AUTO-GENERATED FILE, DO NOT MODIFY!
//

// ignore_for_file: unused_element
import 'package:built_collection/built_collection.dart';
import 'package:vrchat_dart_generated/src/model/current_user.dart';
import 'package:vrchat_dart_generated/src/model/requires_two_factor_auth.dart';
import 'package:built_value/built_value.dart';
import 'package:built_value/serializer.dart';
import 'package:one_of/one_of.dart';

part 'get_current_user200_response.g.dart';

/// GetCurrentUser200Response
///
/// Properties:
/// * [id] - A users unique ID, usually in the form of `usr_c1644b5b-3ca4-45b4-97c6-a2a0de70d469`. Legacy players can have old IDs in the form of `8JoV9XEdpo`. The ID can never be changed.
/// * [requiresTwoFactorAuth] 
@BuiltValue()
abstract class GetCurrentUser200Response implements Built<GetCurrentUser200Response, GetCurrentUser200ResponseBuilder> {
  /// One Of [CurrentUser], [RequiresTwoFactorAuth]
  OneOf get oneOf;

  GetCurrentUser200Response._();

  factory GetCurrentUser200Response([void updates(GetCurrentUser200ResponseBuilder b)]) = _$GetCurrentUser200Response;

  @BuiltValueHook(initializeBuilder: true)
  static void _defaults(GetCurrentUser200ResponseBuilder b) => b;

  @BuiltValueSerializer(custom: true)
  static Serializer<GetCurrentUser200Response> get serializer => _$GetCurrentUser200ResponseSerializer();
}

class _$GetCurrentUser200ResponseSerializer implements PrimitiveSerializer<GetCurrentUser200Response> {
  @override
  final Iterable<Type> types = const [GetCurrentUser200Response, _$GetCurrentUser200Response];

  @override
  final String wireName = r'GetCurrentUser200Response';

  Iterable<Object?> _serializeProperties(
    Serializers serializers,
    GetCurrentUser200Response object, {
    FullType specifiedType = FullType.unspecified,
  }) sync* {
  }

  @override
  Object serialize(
    Serializers serializers,
    GetCurrentUser200Response object, {
    FullType specifiedType = FullType.unspecified,
  }) {
    final oneOf = object.oneOf;
    return serializers.serialize(oneOf.value, specifiedType: FullType(oneOf.valueType))!;
  }

  @override
  GetCurrentUser200Response deserialize(
    Serializers serializers,
    Object serialized, {
    FullType specifiedType = FullType.unspecified,
  }) {
    final result = GetCurrentUser200ResponseBuilder();
    Object? oneOfDataSrc;
    final targetType = const FullType(OneOf, [FullType(CurrentUser), FullType(RequiresTwoFactorAuth), ]);
    oneOfDataSrc = serialized;
    result.oneOf = serializers.deserialize(oneOfDataSrc, specifiedType: targetType) as OneOf;
    return result.build();
  }
}

I dont think its a reasonable ask to require a library change, but @Rexios80 let me know what you think. I have no idea how Dart works in detail, so this might not be as an impactful change as I believe.

Regardless, as there is clearly inconsistent support for oneOf i would rather patch the openapi.yaml in the individual repos that want this. Not sure if it makes sense to keep the PR around, feel free to close.

[Rexios80]

@0xkubectl The built_value generated code is borderline unreadable and also many times the size. I would rather not go back to it.

[Rexios80]

I am almost positive the dart generator does not like this

[0xkubectl]

Lets keep this open until we know how to deal with different capabilities of generators. This exists, and would solve it but it introduces more specs. I.e. a spec for all features (anyOf, oneOf, etc) and their combinations, not sure if that is the way to go. But I am in favor of having a spec that is built with all the features OAPI3 offers for those generators that can deal with it as it means less hacks in consuming libraries etc.

Edit: Im refering to this issue https://github.com/vrchatapi/specification/issues/370

[jellejurre]

C# Doesn't work, as I get:

Unhandled exception. System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.IO.InvalidDataException: The JSON string USER JSON HERE cannot be deserialized into any schema defined. at VRChat.API.Model.GetCurrentUser200Response.FromJson(String jsonString) in vrchatapi-csharp\src\VRChat.API\Model\GetCurrentUser200Response.cs:line 189 This is because currentUserPresence.debugFlag isnt defined, yet without the oneOf, it will decompile just fine and skip the missing field. It seems oneOf makes the deserialization more strict somehow.

[jellejurre]

note, my previous test is on 6.8.0 generator, the latest (7.12.0), doesn't support it at all, and just returns currentUser