amplify-android icon indicating copy to clipboard operation
amplify-android copied to clipboard

Error response when subscribing to "onUpdate" for a model with a repeated owner field.

Open astex opened this issue 1 year ago • 18 comments

How did you install the Amplify CLI?

npm

If applicable, what version of Node.js are you using?

v18.15.0

Amplify CLI Version

11.0.3

What operating system are you using?

Windows

Did you make any manual changes to the cloud resources managed by Amplify? Please describe the changes made.

No manual changes.

Describe the bug

I have a schema like this:

type Baby
  @model
  @auth(rules: [
    {allow: owner, ownerField: "caregivers", operations: [create, read, update, delete]}
  ]) {
    id: ID!
    caregivers: [String]!
    ...
  }

When attempting to subscribe to updates to the model like this (android, kotlin):

Amplify.API.subscribe(
    ModelSubscription.onUpdate(Baby::class.java),
    /* onSubscriptionEstablished = */ { ... },
    /* onNextResponse = */ {
        if (it.errors.isNotEmpty()) {
            Log.e("error response from baby update subscription", it.errors.toString())
        } else {
            ...
        }
    },
    /* onSubscriptionFailure = */ { ... },
    /* onSubscriptionCompleted = */ { ... })

I get this response:

[GraphQLResponse.Error{message='Validation error of type UnknownArgument: Unknown field argument caregivers @ 'onUpdateBaby'', locations='null', path='null', extensions='null'}]

Expected behavior

Log.e should never be called. And, it should wait to call onNextResponse until there is an actual update to the model.

Reproduction steps

  1. Create an API with the schema above.
  2. Try to subscribe as above.

Project Identifier

No response

Log output

No response

Additional information

No response

Before submitting, please confirm:

  • [X] I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.
  • [X] I have removed any sensitive information from my code snippets and submission.

astex avatar Apr 07 '23 19:04 astex

Hi @astex, Thank you for raising this. To ensure that you receive the best possible support, we will move this issue to the Android repository.

AnilMaktala avatar Apr 10 '23 12:04 AnilMaktala

This has sat pending for a while now. Is there a standard SLO for tickets in this repository, so that I can anticipate a resolution? This is a blocker for using amplify for my project.

astex avatar Apr 18 '23 21:04 astex

Thank you for reporting this issue. We are investigating the issue. Can you please share what version of Amplify Android you're using?

eeatonaws avatar May 09 '23 19:05 eeatonaws

I was able to reproduce the issue with the latest version of Amplify Android and am investigating the cause.

eeatonaws avatar May 19 '23 18:05 eeatonaws

Subscriptions for multi-user auth via an array of owners (the caregivers field in your schema) requires AppSync enhanced subscription filtering, which is not currently supported in Amplify Android. Marking this issue as a feature request.

eeatonaws avatar Jun 15 '23 21:06 eeatonaws

+1

siecola avatar Oct 03 '23 15:10 siecola

@eeatonaws Is there any update on this issue? The Amplify Gen 2 documentation does not specify that the multi-user / owners based authorization does not work with subscription.

https://docs.amplify.aws/android/build-a-backend/data/customize-authz/multi-user-data-access/

Thank you!

siecola avatar Jun 28 '24 17:06 siecola

Any update on this ?

marmll41 avatar Aug 14 '24 16:08 marmll41

Thank you for the report. I can see how it is confusing to introduce backend capabilities under a front-end product that does not support the given feature. I will bring this up with our documentation team.

We have not added the capability for this at this time.

tylerjroach avatar Aug 14 '24 16:08 tylerjroach

Can you suggest an alternative solution ? Because we based on swift and javascript only andoird is blocking the prod

marmll41 avatar Aug 14 '24 16:08 marmll41

Hi @marmll41, what does your callsite look like on Swift? Curious how this is working on the other platforms

lawmicha avatar Aug 14 '24 18:08 lawmicha

I found the difference between the Swift and Java library subscription calls. There is an invalid parameter in the Java library subscription call, so I decided to build my own subscription requests on my Android app (using Kotlin), as this creation subscription below:

private fun getOnCreateSlsShoppingListRequest(): GraphQLRequest<SlsShoppingList> {
        val document = """
            subscription OnCreateSlsShoppingList {
                onCreateSlsShoppingList {
                    ${this.attributes}   
                }
            }
        """.trimIndent()
        return SimpleGraphQLRequest(
            document,
            SlsShoppingList::class.java,
            GsonVariablesSerializer()
        )
    }

The this.attributes is just a list of attributes I want to receive in the event. This solves my problem, at least to my use case, where I need my app to subscribe to all entities where the user is present in the owners field.

P.S.: I am using Amplify Gen2 to build my backend.

siecola avatar Aug 14 '24 18:08 siecola

Hi @lawmicha I suppose the Swift solution for this problem is based on these PRs: https://github.com/aws-amplify/amplify-swift/pull/1606 https://github.com/aws-amplify/amplify-swift/pull/3223 (from you) :)

Also, as I understand the missing part in the android library is AuthRuleDecorator.decorateAuthStrategy function and its logic.

Cuz you already participated in the swift PRs, what do you think? Is only AuthRuleDecorator should be implemented similarly to amplify-swift to make it work or there's something that I'm missing?

Thanks!

thetruefixit avatar Aug 15 '24 14:08 thetruefixit

Hi @thetruefixit, thanks for providing those links. Although we may have implemented this in the Swift library, there may be differences in the Android library and implementation of AuthRuleDecorator that this may not be similar or completely straight foward to do. We will have to evaluate the changes further. Do you have the logic is to unblock you? Such as a similar consruction of SimpleGraphQLRequest (like @siecola's comment here) that represents the successful API call you want to make?

lawmicha avatar Aug 19 '24 16:08 lawmicha

@lawmicha Thanks for your response!

Unfortunately, we are limited to using Amplify's DataStore and can't directly call a GraphQL API as in the example (if I understand correctly). To work around this, we tried to extend the AuthRuleRequestDecorator within the "aws-api" module of Amplify-android.

So we edited a bit of it and now we can query data and observe subscriptions, and everything seemed to be working fine. However, after 2-3 minutes of an idle subscriptions, we're encountering many errors for every active subscription that seems to be related to a timeout:

Websocket connection failed. java.io.EOFException ApiException{message=Subscription failed., cause=java.io.EOFException, recoverySuggestion=Check your Internet connection. Is your device online?}

So may I ask, is it a normal behavior for a subscription, cuz the logs do not give a clear answer, just a dropped web socket connection.

Also, is it possible to get some documentation how AuthRuleRequestDecorator should actually work with multi owner support? I'm asking, cuz I want to somehow test that we didn't break anything (btw. library's internal unit-tests are passed with this code)

Thank you in advance! :)

The code snippet, still in development, but works for our subscriptions:


 public <R> GraphQLRequest<R> decorate(
            @NonNull GraphQLRequest<R> request,
            @NonNull AuthorizationType authType
    ) throws ApiException {
        if (!(request instanceof AppSyncGraphQLRequest)) {
            return request;
        }

        AppSyncGraphQLRequest<R> decoratedReqeust = (AppSyncGraphQLRequest<R>) request;
        List<AuthRule> authRules = filterAuthRules(decoratedReqeust.getModelSchema().getAuthRules(), decoratedReqeust.getModelSchema());
        List<AuthRule> ownerRulesListWithReadRestriction = new ArrayList<>();
        Map<String, Set<String>> readAuthorizedGroupsMap = new HashMap<>();

        if (authRules.isEmpty())
            return request;


        boolean publicSubscribeAllowed = false;
        for (AuthRule authRule : authRules) {
            if (doesRuleAllowPublicSubscribe(authRule, authType)) {
                // This rule allows subscribing with the current authMode without adding the owner field, so there
                // is no need to continue checking the other rules.
                publicSubscribeAllowed = true;
                break;
            } else if (isReadRestrictingOwner(authRule)) {
                ownerRulesListWithReadRestriction.add(authRule);
            } else if (isReadRestrictingStaticGroup(authRule)) {
                // Group read-restricting groups by the claim name
                String groupClaim = authRule.getGroupClaimOrDefault();
                List<String> groups = authRule.getGroups();
                Set<String> readAuthorizedGroups = readAuthorizedGroupsMap.get(groupClaim);
                if (readAuthorizedGroups != null) {
                    readAuthorizedGroups.addAll(groups);
                } else {
                    readAuthorizedGroupsMap.put(groupClaim, new HashSet<>(groups));
                }
            }
        }
        
        for (AuthRule ownerBasedRule : ownerRulesListWithReadRestriction) {
            String owner = ownerBasedRule.getOwnerFieldOrDefault();
            SelectionSet selectionSet = appendOwnersIfNeeded(decoratedReqeust.getSelectionSet(), owner);
            if (decoratedReqeust.getOperation().getOperationType() == OperationType.SUBSCRIPTION) {
                if (!publicSubscribeAllowed
                        && userNotInReadRestrictingGroups(readAuthorizedGroupsMap, authType)) {

                    String idClaim = ownerBasedRule.getIdentityClaimOrDefault();
                    String value = getIdentityValue(idClaim, authType);

                    try {
                        decoratedReqeust = decoratedReqeust.newBuilder()
                                .variable(owner, "String!", value)
                                .selectionSet(selectionSet)
                                .build();
                    } catch (AmplifyException error) {
                        // This should not happen normally
                        throw new ApiAuthException(
                                "Failed to set owner field on AppSyncGraphQLRequest.", error,
                                AmplifyException.REPORT_BUG_TO_AWS_SUGGESTION);
                    }
                }
            }
        }

        return decoratedReqeust;
    }

    private SelectionSet appendOwnersIfNeeded(SelectionSet originalSelection, String ownerField) {
        SelectionSet resultSelection = originalSelection;

        boolean hasOwnerField = SelectionSetUtils.findChildByName(resultSelection, ownerField) != null;

        if (!hasOwnerField) {
            resultSelection.getNodes().add(new SelectionSet(ownerField));
        }

        return resultSelection;
    }

thetruefixit avatar Aug 20 '24 15:08 thetruefixit