| [22:00:59] | * polo has joined #farmos |
| [22:09:13] | * polo has quit (Quit: My MacBook has gone to sleep. ZZZzzz…) |
| [05:00:09] | * RafatKhashan[m] has quit (Quit: You have been kicked for being idle) |
| [08:17:36] | <mstenta[m]> | @room farmOS 2.0.0-beta7 has been released! 🎉 https://github.com/farmOS/farmOS/releases/tag/2.0.0-beta7 |
| [09:38:47] | <evered[m]> | ACTION waves again - sunset here - good morning! |
| [09:39:04] | <evered[m]> | *sunrise |
| [09:39:14] | <evered[m]> | ACTION * waves again - sunrise here - good morning! |
| [11:10:42] | <mstenta[m]> | @room Weekly dev call in about 50 minutes: https://meet.jit.si/farmos-dev - All are welcome! |
| [11:20:34] | <mstenta[m]> | paul121: I wonder if we could try to test the simple_oauth 5.2 stuff again |
| [11:20:47] | <mstenta[m]> | I have it all set up locally (along with PHP8) so we can dive in if we want |
| [11:22:00] | <evered[m]> | testing is good - "trust but verify" |
| [11:31:53] | <mstenta[m]> | paul121: I'd also love to talk about `composer.lock` if we have some time :-) |
| [11:53:44] | <paul121[m]> | Both sound good |
| [11:54:25] | <paul121[m]> | I'm having post "upgraded my os" woes... |
| [11:56:08] | <paul121[m]> | Brave/chromium wont load some sites, and won't let me clear browsing data! Arg |
| [11:56:19] | <symbioquine[m]> | Time for Firefox! |
| [11:56:35] | <mstenta[m]> | womp womp |
| [11:56:58] | <mstenta[m]> | i'm gonna jump into the dev call early... nothing else to do 😄 |
| [13:02:54] | <paul121[m]> | symbioquine: re: https://github.com/farmOS/farmOS-map/issues/184 |
| [13:02:54] | <paul121[m]> | I wanted to ask your opinion about the "convenience" method. It might be able to add a convenience method before a larger refactor of using the interaction "active" state? |
| [13:03:40] | <paul121[m]> | Adding the convenience method would introduce the abstraction to however things are implemented, allowing the implementation to change |
| [13:04:14] | <paul121[m]> | But I still need to evaluate how important this even is for the use case I was working on, not sure yet |
| [16:17:50] | <mstenta[m]> | paul121: I am idly starting to run the oauth tests to see if I can get a sense of what's happening |
| [16:21:09] | <FarmerEd[m]> | Oath client is not included in this code by any chance is it? |
| [16:22:51] | <mstenta[m]> | Nothing has changed in 2.0.0-beta7 |
| [16:23:09] | <mstenta[m]> | We're working on upgrading the oauth module ahead of php8 support |
| [16:31:28] | <FarmerEd[m]> | I know that, |
| [16:31:28] | <FarmerEd[m]> | But I possibly need to include an oath client for some modules to authenticate with other software. |
| [16:31:28] | <FarmerEd[m]> | But then again could probably get away with just using the http client already included with Drupal. |
| [16:34:35] | <FarmerEd[m]> | I suppose the opposite of what you are working on. |
| [16:35:00] | <mstenta[m]> | Oooh you mean like what we were talking about with the USDA making a module for the USDA eAuth identity provider? |
| [16:35:13] | <mstenta[m]> | (Similar to something like "login with your google account") |
| [16:38:25] | <FarmerEd[m]> | No not that either, |
| [16:38:25] | <FarmerEd[m]> | I have some node red flows for synchronizing the Department of Agricultures herd database with farmOS but considering migrating to a module. So I'd need my farmOS instance to be the client which connects to the other server via Oauth2 |
| [16:38:38] | <mstenta[m]> | Oooh ok gotcha :-) |
| [16:38:53] | <mstenta[m]> | Yea you're kinda on your own with that. There isn't anything in farmOS that helps with that specifically. |
| [16:39:12] | <mstenta[m]> | But I know paul121 has done some things with similar integrations using OAuth so he might have some pointers |
| [16:39:33] | <FarmerEd[m]> | 👍 |
| [16:40:31] | <FarmerEd[m]> | Cool, I know more or less how it works, just making sure there wasn't anything included before reinventing the wheel |
| [16:42:01] | <mstenta[m]> | I mean... maybe there is something included that I'm not aware of! 😄 |
| [16:42:38] | <FarmerEd[m]> | Think there are a few Drupal client specific modules about too. |
| [16:42:38] | <mstenta[m]> | Guzzle at the very least (that's the PHP requests library basically... comparable to Python `requests` or JS `axios` as I understand it) |
| [16:43:02] | <mstenta[m]> | But there may be OAuth2 specific libraries on top of that that handle flows |
| [16:43:13] | <mstenta[m]> | I'm sure there are... just not sure if it's included in the farmOS code/dependencies |
| [16:44:45] | <FarmerEd[m]> | Yea, I've used Guzzle for a few modules already, and built the oauth2 flow I need in node red using http request nodes so can coble something together I'm sure one way or another. |
| [16:45:03] | <mstenta[m]> | https://oauth.net/code/php/ |
| [16:45:24] | <mstenta[m]> | I imagine this one is what you'd want: https://github.com/thephpleague/oauth2-client |
| [16:45:48] | <mstenta[m]> | Our oauth2 server module uses the opposite package from the same authors |
| [16:45:54] | <mstenta[m]> | (`oauth2-server` instead of `oauth2-client`) |
| [16:46:25] | <mstenta[m]> | > @paul121: I am idly starting to run the oauth tests to see if I can get a sense of what's happening |
| [16:46:25] | <mstenta[m]> | OK so... this response has a `401` status code (which I think we expected - just my first clue): https://github.com/farmOS/farmOS/blob/2.x/modules/core/api/tests/src/Fun... |
| [16:48:09] | <mstenta[m]> | So that method return `NULL`... which means `$token_info['id']` is not set here: https://github.com/farmOS/farmOS/blob/8a2dd9474b4b24cf0cad9cb751edd83405... |
| [16:48:39] | <mstenta[m]> | And that leads to the error we're seeing in the test results (`Undefined array key "id"`) |
| [16:48:57] | <mstenta[m]> | Now... we just need to figure out why the response is resulting in `401`! 😄 |
| [16:52:13] | <paul121[m]> | <mstenta[m]> "But I know paul121 has done some..." <- yep, I've built one OAuth integration but it's just a custom wrapper around the Guzzle client. I wanted to use an oauth-client PHP library but unfortunately their OAuth implementation wasn't 100% to sepc. I've found this is unfortunate & common reality :-/ |
| [16:53:14] | <paul121[m]> | mstenta[m]: yeah, I think at one point we were able to identify we were not getting a token at all |
| [16:53:29] | <paul121[m]> | which could mean many different things :-/ |
| [16:55:43] | <mstenta[m]> | Gonna try stepping into the post request... even though I have no idea what I'm looking at haha |
| [16:56:21] | <mstenta[m]> | (and will undoubtedly lead me through a labyrinth of methods...) |
| [16:58:24] | <FarmerEd[m]> | Is that module on git by any chance paul121: ? |
| [17:02:10] | <paul121[m]> | FarmerEd[m]: yes this farm_farmlab module (the one I wanted to demo!): https://github.com/paul121/farm_farmlab |
| [17:02:10] | <paul121[m]> | This uses the OAuth authorization flow. The client is here, has a couple OAuth helper functions, notably a helper to auto-refresh OAuth tokens which might be useful: https://github.com/paul121/farm_farmlab/blob/1.x/src/FarmLabClient.php |
| [17:03:14] | <mstenta[m]> | Oh that's neat paul121 - I like that it just extends `GuzzleClient` |
| [17:05:25] | <paul121[m]> | Another thing I have learned when creating these "client" classes/services is to create a "ClientFactory" service that injects the credentials/api token/etc separate from your actual client class. The drupal/symfony services have native support for this "factory": |
| [17:05:25] | <paul121[m]> | - https://github.com/paul121/farm_farmlab/blob/1.x/src/FarmLabClientFactor... |
| [17:05:25] | <paul121[m]> | - https://github.com/paul121/farm_farmlab/blob/1.x/farm_farmlab.services.yml |
| [17:08:34] | <FarmerEd[m]> | Cool paul121: that might just be enough to get me started. |
| [17:09:05] | <mstenta[m]> | paul121: ah hmm... maybe a clue: `invalid_client` |
| [17:11:01] | <mstenta[m]> | Or maybe that's not helpful haha - it's interesting that that is the message we were seeing in the original test failures (https://www.drupal.org/project/farm/issues/3282186#comment-14536882) |
| [17:22:47] | <paul121[m]> | ah, that last message is irrelevant. I believe it is fixed with one of our last patches that got included with 5.2.x |
| [17:23:15] | <mstenta[m]> | > paul121: ah hmm... maybe a clue: `invalid_client` |
| [17:23:15] | <mstenta[m]> | this is what i got tracing through the debugger just now |
| [17:23:33] | <mstenta[m]> | will have a better idea shortly... waiting for the functional test to run again and my debugger to stop it :-) |
| [17:23:38] | <mstenta[m]> | brute force debugging |
| [17:26:30] | <mstenta[m]> | happened here: https://github.com/thephpleague/oauth2-server/blob/0f32fbe00b6b1bdd72c8a... |
| [17:26:45] | <mstenta[m]> | gonna figure out why next... |
| [17:31:23] | <mstenta[m]> | ok... |
| [17:33:05] | <mstenta[m]> | I'm in this method: https://git.drupalcode.org/project/simple_oauth/-/blob/5.2.x/src/Reposit... |
| [17:33:12] | <mstenta[m]> | ... and it is returning `FALSE` |
| [17:37:31] | <mstenta[m]> | It's making it into the last check at the bottom (not sure if it should or not), but `$client_secret` is `null`, so it's returning `FALSE` before it can check `$this->passwordChecker->check($client_secret, $client_drupal_entity->get('secret')->value)` |
| [17:40:38] | <mstenta[m]> | I feel like maybe it shouldn't be getting that far? |
| [17:41:09] | <paul121[m]> | mstenta[m]: ah. so the consumer has a secret configured, but no `$client_secret` was provided, so it doesn't need to do a password check |
| [17:41:44] | <mstenta[m]> | ok... |
| [17:43:27] | <mstenta[m]> | so... trying to understand: which block of code do we want to be returning `TRUE` in this test? |
| [17:43:34] | <mstenta[m]> | that last block? or the one before it (which it skips over) |
| [17:44:08] | <mstenta[m]> | (line 71 or line 80?) https://git.drupalcode.org/project/simple_oauth/-/blob/5.2.x/src/Reposit... |
| [17:44:45] | <paul121[m]> | I think those blocks align with the two cases: |
| [17:44:45] | <paul121[m]> | First block = the client does not have a `client_secret` |
| [17:44:45] | <paul121[m]> | Second block = the client does have a `client_secret` |
| [17:44:57] | <mstenta[m]> | ok that makes sense |
| [17:45:03] | <paul121[m]> | so it depends if in the test the client has a secret configured |
| [17:45:21] | <mstenta[m]> | looking... |
| [17:46:09] | <mstenta[m]> | ok so yea... we don't use `client_secret` in any of our tests (if i'm reading correctly) |
| [17:47:19] | <mstenta[m]> | oh wait... |
| [17:47:19] | <paul121[m]> | but do we inherit the client from their tests? |
| [17:47:19] | <mstenta[m]> | ah we doo! yes! |
| [17:47:20] | <mstenta[m]> | yes yes yes |
| [17:47:20] | <mstenta[m]> | got it |
| [17:48:12] | <mstenta[m]> | ah so maybe they started using `client_secret` in the upstream tests... and we need to update ours to send it... (just a guess... looking....) |
| [17:49:37] | <mstenta[m]> | that might be it... running another test... |
| [17:58:57] | <mstenta[m]> | to be continued... dinner time :-) |
| [19:58:45] | <mstenta[m]> | paul121: Alright! Got that test to pass! Now to see if it fixes the others too... 🤞 |