Add federation authentication protocol messages + tests + examples in Scala #1851
Conversation
…ion-protocol-implementation
…ion-protocol-implementation
…ion-protocol-messages
Pull reviewers statsStats of the last 30 days for popstellar:
|
|
|
||
| implicit object FederationChallengeFormat extends JsonFormat[FederationChallenge] { | ||
| final private val PARAM_CHALLENGE_VALUE: String = "value" | ||
| final private val PARAM_CHALLENGE_VALID_UNTIL: String = "validUntil" |
There was a problem hiding this comment.
This field should match the one in the jsonRPC description : "valid_until". If not, when trying to parse a received message, an error will be thrown
| final private val PARAM_ID: String = "laoId" | ||
| final private val PARAM_SERVER_ADDRESS: String = "serverAddress" | ||
| final private val PARAM_OTHER_ORGANIZER: String = "other_organizer" | ||
| final private val PARAM_CHALLENGE: String = "challenge" | ||
|
|
There was a problem hiding this comment.
Same remark as previously. The PARAM should match some keys in the RPC message defined. But once you extracted them, you can use them as you want in your object "FederationExpect". "laoId" -> "lao_id", "serverAddress" -> "server_address", "other_organizer" -> "public_key"
| final private val PARAM_ID: String = "laoId" | ||
| final private val PARAM_SERVER_ADDRESS: String = "serverAddress" | ||
| final private val PARAM_OTHER_ORGANIZER: String = "other_organizer" | ||
| final private val PARAM_CHALLENGE: String = "challenge" |
There was a problem hiding this comment.
Same remark as federationExpect
| final private val PARAM_STATUS: String = "status" | ||
| final private val PARAM_REASON: String = "reason" | ||
| final private val PARAM_ORGANIZER: String = "organizer" | ||
| final private val PARAM_CHALLENGE_MESSAGE = "challenge_message" |
There was a problem hiding this comment.
Same remark as before.
| case JsString(r) => r | ||
| } | ||
|
|
||
| val organizer = jsObj.fields.get(PARAM_ORGANIZER).flatMap(_.convertTo[Option[PublicKey]]) |
There was a problem hiding this comment.
nit : can't the same logic as reason be applied here ?
val organizer = jsObj.fields.get(PARAM_ORGANIZER).collect {
case key @ JsString(_) => key.convertTo[PublicKey]
}
| case challenge_request extends ActionType("challenge_request") | ||
| case challenge extends ActionType("challenge") | ||
| case federation_result extends ActionType("federation_result") |
There was a problem hiding this comment.
If I am not mistaken, string should match actions from jsonRPC format. "federation_result" -> "result"
| other_organizer: PublicKey, | ||
| challenge: Message | ||
| ) extends MessageData { | ||
| require(serverAddress.matches("^(ws|wss):\\/\\/.*(:\\\\d{0,5})?\\/.*$"), s"This is an invalid server address") |
There was a problem hiding this comment.
As mentionned during meeting, using "require" doesn't allow to handle errors gracefully. As you have this pattern on your jsonRPC, you could assume that if a message reaches here, it has the right format. Or you could try to handle it gracefully and return an error/default value in case of wrong format.
…tern check since having it in json schemas ensures that messages have the right format at this level.
| val request_challenge_ = FederationChallengeRequest.buildFromJson(CHALLENGE_REQUEST.toJson.toString) | ||
|
|
||
| request_challenge_ should equal(CHALLENGE_REQUEST) | ||
| } |
There was a problem hiding this comment.
To test that your implementation works with the json examples you could add the json in the /util/examples/json and use readJsonFromPath. That way you can be sure that your implementation is correct with the messages defined in the protocol. Because a subtle bug could be that you are building from a json that you created, so your implementation work to read and write with object within the app, but not for the outside world. You can find some examples in other tests that use readJsonFromPath
be2-scala/src/main/scala/ch/epfl/pop/model/network/method/message/data/ActionType.scala
Show resolved
Hide resolved
...c/main/scala/ch/epfl/pop/model/network/method/message/data/federation/FederationResult.scala
Outdated
Show resolved
Hide resolved
be2-scala/src/main/scala/ch/epfl/pop/model/objects/Base16Data.scala
Outdated
Show resolved
Hide resolved
|
|
|
|
|
This PR defines :
It also includes examples of each message and some unit tests