Use of InsecureConnectionBuilder without checking for allowInsecureConnections

[hschaeufler]

I have just looked at the source code of FlutterAppauthPlugin.java. One place that seems strange to me is line 98. https://github.com/MaikuB/flutter_appauth/blob/0d96bcd0f0fb4924ac290f7170d84310d4a0ed9e/flutter_appauth/android/src/main/java/io/crossingthestreams/flutterappauth/FlutterAppauthPlugin.java#L98C1-L100C53

    AppAuthConfiguration.Builder authConfigBuilder = new AppAuthConfiguration.Builder();
    authConfigBuilder.setConnectionBuilder(InsecureConnectionBuilder.INSTANCE);
    authConfigBuilder.setSkipIssuerHttpsCheck(true);
    insecureAuthorizationService =
        new AuthorizationService(applicationContext, authConfigBuilder.build());

In other places where the InsecureConnectionBuilder is used, it is checked beforehand whether you want to allow insecure connections. Here is an excerpt from line 708.

        AppAuthConfiguration.Builder authConfigBuilder = new AppAuthConfiguration.Builder();
        if (allowInsecureConnections) {
          authConfigBuilder.setConnectionBuilder(InsecureConnectionBuilder.INSTANCE);
          authConfigBuilder.setSkipIssuerHttpsCheck(true);
        }

Shouldn't that also be the case here? Or am I wrong?

[MaikuB]

It's not relevant in the first instance of the code snippet as the context for that entire method is to create two instances of an AuthorizationService, one that goes through a secure connection and one that doesn't. At this point it doesn't matter what value allowInsecureConnections has as that is used later on. If anything, it looks like you came across a part of the code that's in the second snippet that wasn't cleaned up to pick which instance of the AuthorizationService to use. This is probably where the confusion came about and will look to clean up

[hschaeufler]

It's not relevant in the first instance of the code snippet as the context for that entire method is to create two instances of an AuthorizationService, one that goes through a secure connection and one that doesn't. At this point it doesn't matter what value allowInsecureConnections has as that is used later on. If anything, it looks like you came across a part of the code that's in the second snippet that wasn't cleaned up to pick which instance of the AuthorizationService to use. This is probably where the confusion came about and will look to clean up

Ah, I had overlooked that. Thank you for the quick clarification and the refactoring that makes it clearer for me. :)