konsultaner / connectanum-dart

This is a WAMP client (Web Application Messaging Protocol) implementation for the dart language and flutter projects.
MIT License
22 stars 14 forks source link

support dynamic realm assignment and server-side authextra #24

Closed oberstet closed 3 years ago

oberstet commented 3 years ago

this is almost complete (authextra: couldnt figure out the syntax adhoc .. see FIXME):

Bildschirmfoto von 2021-03-20 02-13-04

this is what the server sends:

Accept(realm=<member-93538308-8a43-43c5-a411-ae3b4bdd5bd7>, authid=<member-93538308-8a43-43c5-a411-ae3b4bdd5bd7>, authrole=<anonymous>, authmethod=cryptosign, authprovider=function, authextra={'member_oid': '93538308-8a43-43c5-a411-ae3b4bdd5bd7', 'credential_oid': '8c9b887c-95a0-4b2d-812e-7ee5bc9f91ce'})

another note: Map<String, String> authExtra doesn't actually cut it: the authextra is a dict with string keys, but arbitrary values .. eg nested objects, arrays, ..

codecov-io commented 3 years ago

Codecov Report

Merging #24 (1de730c) into master (e3a7649) will decrease coverage by 0.12%. The diff coverage is 72.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #24      +/-   ##
==========================================
- Coverage   92.89%   92.76%   -0.13%     
==========================================
  Files          40       40              
  Lines        2013     2019       +6     
==========================================
+ Hits         1870     1873       +3     
- Misses        143      146       +3     
Impacted Files Coverage Δ
lib/src/serializer/msgpack/serializer.dart 97.93% <50.00%> (-0.77%) :arrow_down:
lib/src/message/details.dart 100.00% <100.00%> (ø)
lib/src/protocol/session.dart 88.23% <100.00%> (+0.14%) :arrow_up:
lib/src/serializer/json/serializer.dart 96.69% <100.00%> (+<0.01%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e3a7649...1de730c. Read the comment docs.

oberstet commented 3 years ago

ok, mmmh, right: the tests fail because the current classes do not distinguish between what the client requested, and what the server actually assigned ... 2 different things.

I don't know how you would like to have it ... but without knowing that, doesnt make sense for me to fix the tests ...

konsultaner commented 3 years ago

another note: Map<String, String> authExtra doesn't actually cut it

Dart has a big issue. It does not support reflections. Its impossible to serialize non pre-analyzed class definitions. So I decidided on a shallow version. I need to think about a more generic version here. I got some Ideas but havn't had time for it yet. This is not just an authextra problem. ..🙄

konsultaner commented 3 years ago

ok, mmmh, right: the tests fail because the current classes do not distinguish between what the client requested, and what the server actually assigned ... 2 different things.

I don't know how you would like to have it ... but without knowing that, doesnt make sense for me to fix the tests ...

Ill digg into it asap.

oberstet commented 3 years ago

^ ok, thanks for commenting! sad. the language seems kinda broken .. also in general. my impression after using it a bit. but anyways.

could I ask for one more comment rgd https://github.com/oberstet/scratchbox/blob/16683583a06685f670f0f430f831d308d9727700/flutter/ex1/test_client_wampcs.dart#L52

I feel like I'm missing sth .. and doing everything (eg rpc) with callbacks and nesting .. would like to avoid that ..

oberstet commented 3 years ago

another note: had to switch to msgpack (fixed a bug along the way), since I figure https://github.com/wamp-proto/wamp-proto/blob/master/rfc/text/basic/bp_binary_conversion_of_json.md isn't supported with json, right? I need binary values in the rpcs/pubsub ..

konsultaner commented 3 years ago

another note: had to switch to msgpack (fixed a bug along the way), since I figure https://github.com/wamp-proto/wamp-proto/blob/master/rfc/text/basic/bp_binary_conversion_of_json.md isn't supported with json, right? I need binary values in the rpcs/pubsub ..

Right. I totally forgot about that. My Router actually supports that. Could you add a new issue on this?

konsultaner commented 3 years ago

I feel like I'm missing sth .. and doing everything (eg rpc) with callbacks and nesting .. would like to avoid that ..

If you hate the nesting, you may just use the await instead of the listen(). You should read about dart streams to clear this out. This has been implemented quite nicely.

I tried to stick as close as possible to the dart stream model as possible to make package integrate seamlessly into flutter, that makes heavy use of streams. It uses streams to make the data binding magic happen.

oberstet commented 3 years ago

Could you add a new issue on this?

https://github.com/konsultaner/connectanum-dart/issues/25

If you hate the nesting, you may just use the await instead of the listen(). You should read about dart streams to clear this out. This has been implemented quite nicely.

I should read the manual (Dart) of course;) sorry, I didn't, tried to get away with copy-paste, but this streams feature seems to be a Dart thing. I guess I could also adapt some example code .. that would actually be really nice to have. The only one is https://github.com/konsultaner/connectanum-dart/blob/master/example/main.dart, right? Unfort., doesn't show RPC calls with await ..

But yes, you're right: RTFM;) my "problem". unfort., I don't have the time to properly learn Dart ...

oberstet commented 3 years ago

ok, I've read a bit about Dart streams. When I get this correctly, a remote proc. call returns a "single-subscriber stream" (not a broadcast stream)? and streams have a .first method that returns a Future that can be awaited. And the value is a Result which has arguments for the positional return values?

if this is the case, I don't understand why

final result = await session1.call('xbr.network.is_member', arguments: [ethkey_adr]).first;
final bool is_member = result.arguments[0];
print(is_member);

doesn't work (it will not print anything, and subsequent code is never executed).

could you give me a tip what I'm doing wrong?

konsultaner commented 3 years ago

I cant see the issue. This should actually work. As soon as I get the time, I will debug it.

oberstet commented 3 years ago

thanks a lot! if the above syntax is actually supposed to work, that is indeed nice syntax. I guess the stream would also allow me not only do .first for a regular call, but actually process more results in progressive result RPCs?

oberstet commented 3 years ago

not sure about "project policy": I merged your recent changes (JSON binary) into this PR. do you want to have PRs rebased to master (and possibly squashed) or is this ok?

konsultaner commented 3 years ago

not sure about "project policy": I merged your recent changes (JSON binary) into this PR. do you want to have PRs rebased to master (and possibly squashed) or is this ok?

I don't know. I usually don't get PRs for my projects 😅. Just do what you think is best. You are more experienced.

thanks a lot! if the above syntax is actually supposed to work, that is indeed nice syntax. I guess the stream would also allow me not only do .first for a regular call, but actually process more results in progressive result RPCs?

exactly, the cool thing is that you can put streams in a for loop. Which would work with progressive call results. so

await for (var result in callStream) {
  print(result);
}
konsultaner commented 3 years ago

if this is the case, I don't understand why

final result = await session1.call('xbr.network.is_member', arguments: [ethkey_adr]).first;
final bool is_member = result.arguments[0];
print(is_member);

doesn't work (it will not print anything, and subsequent code is never executed).

Is this still an issue?

konsultaner commented 3 years ago

I think this

session.id = message.sessionId;
session.realm = message.details.realm;
session.authId = message.details.authid;
session.authMethod = message.details.authmethod;
session.authProvider = message.details.authprovider;
session.authRole = message.details.authrole;
session.authExtra = message.details.authextra;

should be changed to this:

session.id = message.sessionId;
session.realm = message.details.realm ?? session.realm; // <--- otherwize it may get set to null with wampcra or wampscram
session.authId = message.details.authid;
session.authMethod = message.details.authmethod;
session.authProvider = message.details.authprovider;
session.authRole = message.details.authrole;
session.authExtra = message.details.authextra;
konsultaner commented 3 years ago

your msgpack bugfix crashes the tests. The guy that contributed msgpack was obviously not writing the tests carefully enough. I should have checked them more detailed before merging...

oberstet commented 3 years ago

Is this still an issue?

yes

exactly, the cool thing is that you can put streams in a for loop.

that sounds great! but as said, it doesn't actually work ... for me at least. I merged everything you have on master (I still need to run my branch, because of the fixes there), but it still doesn't return nor make progress beyond that line then.

https://github.com/oberstet/scratchbox/blob/dfee0e39e61b6a35dd95d09e3f41197279f58ab7/flutter/ex1/test_client_wampcs.dart#L56

Bildschirmfoto von 2021-03-21 00-49-16

oberstet commented 3 years ago

should be changed to this:

I don't think so: the realm must always be returned by the router. If not, that's a bug in the router.

yes, that change makes the unit test pass .. because the unit test test/client_test.dart is also buggy;)

https://github.com/konsultaner/connectanum-dart/pull/24/commits/6d5c58b69418d4dd85e732f8609373d5154cb4ad

oberstet commented 3 years ago

rgd the failing MsgPack tests, pls see my comment https://github.com/konsultaner/connectanum-dart/pull/24/commits/1de730c717715e62b584d375f6d60b86d6cdc727

oberstet commented 3 years ago

ok, now the tests pass. I don't care about codecov, sorry, IMO that should not prevent merging. the coverage is less, but the disabled tests were wrong anyways ...

(cpy392_1) oberstet@intel-nuci7:~/scm/oberstet/connectanum-dart$ dart test
00:13 +80: test/transport/websocket/websocket_transport_io_test.dart: WebSocket protocol with io communication Opening a server connection and simple send receive scenario using a serializer
Received protocol wamp.2.json
Received protocol wamp.2.msgpack
00:29 +103: All tests passed!                                      
liquidiert commented 3 years ago

I've provided the MessagePack implementation and tests for it but I guess I fucked up 😅 though the tests worked for me and test input was generated with a valid python implementation of msgpack. Did I translate the messages wrong?

oberstet commented 3 years ago

@liquidiert no worries ... only 1 bug: HELLO.authextra is not a list, but a dict (if present)

also, I would have fixed the tests as well, but

a) the general test approach is dubious .. see my comment https://github.com/konsultaner/connectanum-dart/commit/1de730c717715e62b584d375f6d60b86d6cdc727 and b) I have no clue about Dart and basically just copy-paste stuff;)


the fact that streams with RPCs don't work for me is kinda annoying, because it leads to code like this:

https://github.com/oberstet/scratchbox/blob/8f9ca85e4694a7628db8fef75e093cf04495e64b/flutter/ex1/test_client_wampcs.dart#L48

currently, I'm stuck on: "how do I access map items in Dart"? ;) all newbie stuff, sure, I don't have the time ..

https://github.com/oberstet/scratchbox/blob/8f9ca85e4694a7628db8fef75e093cf04495e64b/flutter/ex1/test_client_wampcs.dart#L71

but in principle, it works:

Bildschirmfoto von 2021-03-21 01-44-47

oberstet commented 3 years ago

It seems what I get with result.arguments[0] is not a map, but a generic Dart "object". So how do I cast this to a map? Because it is a map, definitely.

              print(result.arguments[0].containsKey('oid'));
              print(result.arguments[0]['oid']);

runs into

(cpy392_1) oberstet@intel-nuci7:~/scm/oberstet/scratchbox/flutter/ex1$ make run 
dart test_client_wampcs.dart
test_client_wampcs.dart:72:41: Error: The method 'containsKey' isn't defined for the class 'Object'.
 - 'Object' is from 'dart:core'.
Try correcting the name to the name of an existing method, or defining a method named 'containsKey'.
              print(result.arguments[0].containsKey('oid'));
                                        ^^^^^^^^^^^
test_client_wampcs.dart:73:40: Error: The operator '[]' isn't defined for the class 'Object'.
 - 'Object' is from 'dart:core'.
Try correcting the operator to an existing operator, or defining a '[]' operator.
              print(result.arguments[0]['oid']);
                                       ^
Makefile:6: recipe for target 'run' failed
make: *** [run] Error 254
liquidiert commented 3 years ago

I'm not entirely sure but the result should be an object of the Result class, thus you can access the oid like result.argumets[0].oid. And yeah js listener syntax is awful but as @konsultaner already mentioned you can just await the result :)

liquidiert commented 3 years ago

Also dart casting can be achieved via the as keyword or from factory constructors if they are provided!

liquidiert commented 3 years ago

Regarding the MsgPack tests I'll try to fix / improve them tomorrow; also jfyi @konsultaner

oberstet commented 3 years ago

thanks for helping me!

awaiting the result is what I tried, but it doesn't work:

    // This does not work .. why?
    final result = await session1.call('xbr.network.is_member', arguments: [ethkey_adr]).first;
    print('RPC result: ${result}');

accessing the oid like you posted: tried that, doesn't work:

(cpy392_1) oberstet@intel-nuci7:~/scm/oberstet/scratchbox/flutter/ex1$ make run 
dart test_client_wampcs.dart
test_client_wampcs.dart:74:41: Error: The getter 'oid' isn't defined for the class 'Object'.
 - 'Object' is from 'dart:core'.
Try correcting the name to the name of an existing getter, or defining a getter or field named 'oid'.
              print(result.arguments[0].oid);
                                        ^^^
Makefile:6: recipe for target 'run' failed
make: *** [run] Error 254

while

print(result.arguments[0]);

does show that the result is what I expect - I just cannot process it properly:

{oid: [147, 83, 131, 8, 138, 67, 67, 197, 164, 17, 174, 59, 75, 221, 91, 215], address: [227, 226, 94, 163, 69, 56, 31, 165, 205, 134, 113, 92, 120, 214, 77, 136, 4, 215, 220, 245], level: 1, profile: QmUEM5UuSUMeET2Zo8YQtDMK74Fr2SJGEyTokSYzT3uD94, eula: QmRgTwgj9b4j2DMZ9iGWkkFGY4gT6VBwYEE8KAfueE3S8p, balance: {xbr: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], eth: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]}, email: tobias.oberstein@gmail.com, username: oberstet, created: 1614827037973698512, markets: 0, catalogs: 0, domains: 0}

Why on earth did Google had to invent a broken language with no gain vs existing languages, only pain? Wtf.

liquidiert commented 3 years ago

Yeah just had a look at the arguments list definition and it's a list of Objects. You could try to cast it to a dynamic type (which should have the same effect as the any type in typescript). Guess it's the "old" I'm dynamic but also got types problem 😬

oberstet commented 3 years ago

alright, I finally figured it out:

// cast the generic Dart object into a dynamic map. awesome. whatever. this is required:
final Map<dynamic, dynamic> m = result.arguments[0];

// case the items we care about
final String oid = Uuid.unparse(m['oid']);
final String address = hex.encode(m['address']);
final int level = m['level'];
final String profile = m['profile'];
final String eula = m['eula'];
final String email = m['email'];
final String username = m['username'];
print('oid=${oid}, address=${address}, level=${level}, profile=${profile}, eula=${eula}, email=${email}, username=${username}');

This is ridiculous. Worse than Java - and that sucks big time already. Anyways, Dart is not for me. Looking forward getting back home to a real language (Python) soon=) rant over.

Bildschirmfoto von 2021-03-21 02-50-28

liquidiert commented 3 years ago

Get your point 😬 maybe changing the type of the args list to dynamic would help @konsultaner?

konsultaner commented 3 years ago

I could make the arguments a List<dynamic> and the argumentsKeywords a Map<dynamic,dynamic>. I'm just a fan of correct typing, but since dart with flutter has no reflections and there are no perfomence issues with the payload serialization, taking the dynamic type would be a good way to ease things up. 🤔

oberstet commented 3 years ago

after understanding that Dart is actually Java reinvented (both syntax and semantics), and since

dart with flutter has no reflections

I agree with

taking the dynamic type would be a good way to ease things up

In Java, because there is proper reflection, and because there is an actually quite awesome library shielding away all the details

https://github.com/FasterXML/jackson

this allows Autobahn|Java to support transparent handling of POJOs.

eg consider

https://github.com/crossbario/autobahn-java/blob/master/demo-gallery/src/main/java/io/crossbar/autobahn/demogallery/data/Person.java

and

https://github.com/crossbario/autobahn-java/blob/master/demo-gallery/src/main/java/io/crossbar/autobahn/demogallery/POJOCallExamples.java https://github.com/crossbario/autobahn-java/blob/master/demo-gallery/src/main/java/io/crossbar/autobahn/demogallery/POJORegisterExamples.java


with regard to strong typing discipline with WAMP, besides run-time reflection + POJO auto-mapping, the other road is IDL.

eg here is a WAMP API defined

https://github.com/crossbario/xbr-app/blob/4afd0f6039c75619f27db1b054796a0190bf6398/schema/climate.fbs#L70

This is "beta", but Autobahn|Python includes a command line tool that can read such schemata and generate Python bindings - both dynamically typed (any serializer with generated run-time type checking) and statically types: application payload is actually Flatbuffers and zero-copy.

This is what we actually use ourself recently.

Can't explain all that in a comment here, but suffice to say, when using a Python IDE with Python type hint support, this provides super convenient typed access, eg

Bildschirmfoto von 2021-03-21 09-42-43

This is an event handler. Similar for publish, call, register.

It doesn't stop there. Using the exact same schema, one can use those types in zLMDB database tables!

@table('01760ab3-9d72-4aa2-8968-2bba556092fc', build=CryptosignCredential.build, cast=CryptosignCredential.cast)
class CryptosignCredentials(MapUuidFlatBuffers):
    pass

@table('89c50879-bf8c-452a-b212-741295365dd2')
class IndexCryptosignCredentialsByPubkey(MapBytes32Uuid):
    pass

which can be used like

with self._db.begin() as txn:
    cred_oid = self._schema.idx_credentials_by_pubkey[txn, _pubkey]
    if cred_oid:
        cred = self._schema.credentials[txn, cred_oid]
        if cred:
            auth = cred.marshal()

So using 1 schema definition allows to define/generate both WAMP API and DB tables. Anyways, too much for a comment;)

konsultaner commented 3 years ago

In Java, because there is proper reflection, and because there is an actually quite awesome library shielding away all the details https://github.com/FasterXML/jackson this allows Autobahn|Java to support transparent handling of POJOs.

This is how I do it in connectanum too.

rgd schema using. I ususally work with schemaless databases so this is not needed for me. but defining a schema would probably help to overcome the non-reflection issue in dart or flutter, resp.. I think the lack of reflections is hard discussed and will most probably not change in the future.

konsultaner commented 3 years ago

@oberstet with connectanum-dart 1.1.4 you can do:

print('''
    oid=${Uuid.unparse(result.arguments[0]['oid'])}, 
    address=${hex.encode(result.arguments[0]['address'])}, 
    level=${result.arguments[0]['level']}, 
    profile=${result.arguments[0]['profile']}, 
    eula=${result.arguments[0]['eula']}, 
    email=${result.arguments[0]['email']}, 
    username=${result.arguments[0]['username']}
''');

I changed the signature to use dynamic.

oberstet commented 3 years ago

This is how I do it in connectanum too.

ok, then how can I receive eg a call result into a typed object? eg an instance of user defined class like

https://github.com/crossbario/autobahn-java/blob/master/demo-gallery/src/main/java/io/crossbar/autobahn/demogallery/data/Person.java

it seems to me, that would require sth like https://github.com/k-paxian/dart-json-mapper ?


with connectanum-dart 1.1.4 you can do:

oh, nice! much appreciated! actually, could you merge this PR as well? to consolidate what we have now, and so that I can point collegues to the respective release?

wamp-cryptosign and dynamic realm assignment works now (tested against crossbar and our endpoint)

lets discuss flatbuffers/schema/idl on a new issue?

to begin with, Dart has first class flatbuffers support https://google.github.io/flatbuffers/flatbuffers_guide_use_dart.html ..

oberstet commented 3 years ago

btw, I also tested both JSON and MsgPack: now works both (identical results/behavior). +1

konsultaner commented 3 years ago

lets discuss flatbuffers/schema/idl on a new issue?

Yes sure, but:

There currently is no support for parsing text (Schema's and JSON) directly from Dart, though you could use the C++ parser through Dart Native Extensions. Please see the C++ documentation for more on text parsing (note that this is not currently an option in Flutter - follow this issue for the latest).

konsultaner commented 3 years ago

ok, then how can I receive eg a call result into a typed object? eg an instance of user defined class like

Sorry for the confusion. I tried to say that I implemented the Jackson thing in my Java Router implementation just like that.

oberstet commented 3 years ago

so what is missing to merge this PR?

oberstet commented 3 years ago

There currently is no support for parsing text (Schema's and JSON) directly from Dart, though you could use the C++ parser through Dart Native Extensions.

what are you talking about?

aaah, you mean the section "Text Parsing" on the Flatbuffers site?

this does not matter at all because

now, the only thing we would need to support Dart are Jinja2 template file flavors for Dart rather than Python ..

konsultaner commented 3 years ago

so what is missing to merge this PR?

I just want to read trough the code. I'm only on my phone atm. Ill merge it as soon as I'm back at my Computer. 😁

konsultaner commented 3 years ago

There currently is no support for parsing text (Schema's and JSON) directly from Dart, though you could use the C++ parser through Dart Native Extensions.

what are you talking about?

This is a quote from the flatbuffer website. They seem to have problems with reflections as well.

oberstet commented 3 years ago

This is a quote from the flatbuffer website. They seem to have problems with reflections as well.

yeah, I figured and modified my comment.

in short: no worries. we don't need to do this in Dart.

as said, this is a bit of a head fuck .. I can explain it in more detail on a new issue .. the crucial point to understand is https://github.com/google/flatbuffers/blob/master/reflection/reflection.fbs

konsultaner commented 3 years ago

Ah ok, got it. .. kind of 😅 new issue would be nice.

Just one more thing. I'm experimemting on crosscompiling c++ sources with zig compiler. This way I could use c++ code on all supported dart platforms including web.

oberstet commented 3 years ago

kind of sweat_smile new issue would be nice.

https://github.com/konsultaner/connectanum-dart/issues/26

this will be long text, and probably wrong to have it as an issue here. sorry for occupying your issue tracker. the "issue" is actually a bigger story ..

konsultaner commented 3 years ago

I'll merge it that you get quick access for your colleges, but I'm going to have to fix the "FIXME" befor final release in the pub.dev.

konsultaner commented 3 years ago

another note: Map<String, String> authExtra doesn't actually cut it

@oberstet for all auth methods known to me authextra has always used String for values. This was the reason why I used Map<String, String>. You probably see Map<String, dynamic> here?

konsultaner commented 3 years ago

it is also wrong, as for neither JSON nor MsgPack there is a bijection between values and their encodings (such as for example with bencode)

This is true, but I want to make sure that all values are respected to at least be a part of the result. I'll fix it ;)