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

Updating gateway doc for authentication support #165

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

notthetup
Copy link
Collaborator

With the added support for "auth" (Authentication/Authorization) in fjage's protocol, I have updated the Gateway.md document which documents the requirements for Gateways.

Summary :

  • added an authenticate method on the Gateway
  • receive method will return either the Message|Error|null where the Error(AuthorizationError) can be returned if there was an {auth: false} when a certain message was received.
  • agentForService/agentsForService may also receive an {auth: false} and must return Error in that case

Discussion :

  • How to identify which messages received an {auth: false}, especially if multiple messages are sent.
  • When using send, and the messages gets an {auth: false} response, it will be ignored. Is that OK?

@notthetup notthetup self-assigned this Jan 22, 2021
- Tries to authenticate the Gateway connection with the master container.
- Must send the credential string using command `{"action":"auth","creds":"boo"}`
- Must wait for a the first `{"auth": true}` or `{"auth": false}` received from the master and return the result.
- Must default timeout to 1000 millisecond.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is 1000 ms a MUST?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Will change to a suggestion.

### `getAgentID()` :: Void -> AgentID
### `authenticate` :: String credentials -> Boolean
- Tries to authenticate the Gateway connection with the master container.
- Must send the credential string using command `{"action":"auth","creds":"boo"}`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a comment on what creds may contain?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would depend on the implementation of the Firewall that is being used.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but just a comment on this to state so?

- Sends a _Message_ to the recipient indicated in the message.

### `receive()` :: (Object filter), (Int timeout) -> Message
### `receive()` :: (Object filter), (Int timeout) -> Message | Error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

returning Error makes sense for some languages, other places it is throwing error. Do we need to define it as a return value?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in #167, can't do this generally.

@@ -37,6 +43,7 @@ All fjåge Gateway implementations should implement the following classes and me

### `request()` :: Message, (Int timeout) -> Message
- Sends a request and waits for a response.
- May return an `Error` of type `AuthorizationError` if a `{"auth": false}` is seen while waiting for the response
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to go update all gateways to throw exceptions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in #167, can't do this generally.

@notthetup notthetup marked this pull request as draft January 25, 2021 08:57
@notthetup notthetup force-pushed the dev branch 2 times, most recently from 39da9e2 to 38b02e9 Compare July 7, 2021 04:10
Base automatically changed from dev to master February 20, 2023 23:38
@notthetup notthetup force-pushed the master branch 2 times, most recently from 28154c5 to 1dabc1f Compare January 31, 2025 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants