Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

notify verb should work on authenticated connections only #1225

Open
sitaram-kalluri opened this issue Feb 27, 2023 · 1 comment
Open

notify verb should work on authenticated connections only #1225

sitaram-kalluri opened this issue Feb 27, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@sitaram-kalluri
Copy link
Member

sitaram-kalluri commented Feb 27, 2023

Describe the bug

  • In notify.dart the requireAuth method returns false. This is a bug because the notifications can be sent only on an authenticated connection. So, the requireAuth method should return true.

  • In the AbstractVerbHandler.dart, the below condition does not include pol authenticated connections. So, returning "true" from requireAuth might break the working of the notify verb. Modify the below code snippet to process the verb on the pol-authenticated connections.

if (getVerb().requiresAuth() && !atConnectionMetadata.isAuthenticated) {
      throw UnAuthenticatedException('Command cannot be executed without auth');
    }

Steps to reproduce

  1. Connection to a secondary server via the terminal
  2. Run notify verb (without performing authentication)
  3. Notify verb returns "data:null"

Expected behaviour

The notify verb should work on authentication connections only. When attempting to run on an unauthenticated connection an exception should be returned.

@sitaram-kalluri sitaram-kalluri added the bug Something isn't working label Feb 27, 2023
@gkc
Copy link
Contributor

gkc commented Mar 6, 2023

A better solution here might be to overhaul the verb permission structure so instead of 'requiresAuth' we flip it to say whether or not the owner, or another atServer, or unauthenticated, can execute the verb.

Also: This isn't important right now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants