Another click in confirm account results in error

wolak88 commented 3 years ago

Describe the bug

After user confirms account, second click on the link errors with:

{
  data: null,
  errors: [
    {
      message: "Invalid confirmation token. Please try again",
      locations: [
      {
        line: 1,
        column: 44
      }
      ],
      path: [
        "userConfirmAccount"
      ],
      extensions: {
        code: "USER_ERROR"
      }
    }
  ]
}

Environment

gem "graphql_devise", "~> 0.15"

Steps to reproduce

Create new user account -> click confirmation link -> click in the link again

Expected behavior

If the user confirmed account just redirect to the redirect url.

Actual behavior

Returns graphql error

mcelicalderon commented 3 years ago

Right, that might actually be better. I'll take a closer look and check if there's any reason not to do that. We will soon add another confirmation flow where the link on the email will take you directly to the client application, so confirming the account can be handled there by providing the token on the mutation. It would be easier to handle an invalid or non existent token error there, plus the link on the email would always take you somewhere instead of getting stuck on a JSON response screen

wolak88 commented 3 years ago

Thanks, looking forward to it, atm I'm going to override this resolver and call super if user still needs confirmation. Thanks @mcelicalderon

mcelicalderon commented 3 years ago

That should work just fine. I would maybe just advise to be careful if you are whitelisting urls as a safety measure. Maybe call this line before redirecting as anyone could resend confirmation via API and redirect to a malicious site.

wolak88 commented 3 years ago

I have one issue and maybe you can point me in the right direction. I have created query to overwrite the default one, but when I'm calling it I get:

Failed to build return type for Query.userConfirmAccount from nil: (RuntimeError) Unexpected type input: nil (NilClass)

ruby/gems/3.0.0/gems/graphql-1.12.8/lib/graphql/schema/member/build_type.rb:88:in `parse_type'
ruby/gems/3.0.0/gems/graphql-1.12.8/lib/graphql/schema/field.rb:521:in `type'
ruby/gems/3.0.0/gems/graphql-1.12.8/lib/graphql/schema/field.rb:140:in `connection?'
ruby/gems/3.0.0/gems/graphql-1.12.8/lib/graphql/schema/field.rb:300:in `initialize'
ruby/gems/3.0.0/gems/graphql-1.12.8/lib/graphql/schema/member/accepts_definition.rb:142:in `initialize'
ruby/gems/3.0.0/gems/graphql-1.12.8/lib/graphql/schema/field.rb:126:in `new'
ruby/gems/3.0.0/gems/graphql-1.12.8/lib/graphql/schema/field.rb:126:in `from_options'
ruby/gems/3.0.0/gems/graphql-1.12.8/lib/graphql/schema/member/has_fields.rb:12:in `field'
ruby/gems/3.0.0/gems/graphql_devise-0.15.0/lib/graphql_devise/resource_loader.rb:58:in `block in call'
ruby/gems/3.0.0/gems/graphql_devise-0.15.0/lib/graphql_devise/resource_loader.rb:57:in `each'
ruby/gems/3.0.0/gems/graphql_devise-0.15.0/lib/graphql_devise/resource_loader.rb:57:in `call'
ruby/gems/3.0.0/gems/graphql_devise-0.15.0/lib/graphql_devise/schema_plugin.rb:122:in `block in load_fields'
ruby/gems/3.0.0/gems/graphql_devise-0.15.0/lib/graphql_devise/schema_plugin.rb:119:in `each'
ruby/gems/3.0.0/gems/graphql_devise-0.15.0/lib/graphql_devise/schema_plugin.rb:119:in `load_fields'
ruby/gems/3.0.0/gems/graphql_devise-0.15.0/lib/graphql_devise/schema_plugin.rb:18:in `initialize'
app/graphql/trade_hub_schema.rb:2:in `new'
app/graphql/trade_hub_schema.rb:2:in `<class:XYZSchema>'

Query:

module Queries
  class UserConfirmAccount < GraphqlDevise::Resolvers::ConfirmAccount
    def resolve(confirmation_token:, redirect_url:)
      super
    end
  end
end

It looks like it breaks whole schema as even introspection query stops working.

mcelicalderon commented 3 years ago

You will need to set a type for your resolver

module Queries
  class UserConfirmAccount < GraphqlDevise::Resolvers::ConfirmAccount
    type Types::UserType, null: true # Using user as an example for your authenticatable resource

    def resolve(confirmation_token:, redirect_url:)
      super
    end
  end
end

that is always required when you override a resolver or mutation. Well, it's a bit different for mutations as you can optionally define fields for the mutation return type.

wolak88 commented 3 years ago

Ahhh my bad I've tried with:

field Types::UserType, null: true

Thanks!

mcelicalderon commented 3 years ago

v0.17.0 was released and now you can use the new register mutation that will send an email with a link to your client app and a confirmation token. Then it's up to you to call the confirmation mutation from your client app and handle errors if any (e.g. account was already confirmed).

graphql-devise / graphql_devise