Telodendria/Telodendria
Telodendria
/
Telodendria
3
6
Fork 7
Code Issues 8 Pull Requests 1 Actions Projects 3 Releases 4 Wiki Activity

Implement GET /_telodendria/admin/tokens #37

Closed
lda wants to merge 13 commits from lda/telodendria:implement-26 into master
pull from: lda/telodendria:implement-26
merge into: Telodendria:master
Conversation 5 Commits 13 Files Changed 7 +285
lda commented 2023-10-29 08:35:45 -05:00
Contributor
Copy Link

Implements everything in #26 :

  • GET /_telodendria/admin/tokens
  • GET /_telodendria/admin/tokens/[token]
  • POST /_telodendria/admin/tokens
  • DELETE /_telodendria/admin/tokens/[token]

Please review the developer certificate of origin:

  1. The contribution was created in whole or in part by me, and I have
    the right to submit it under the open source licenses of the
    Telodendria project; or
  2. The contribution is based upon a previous work that, to the best of
    my knowledge, is covered under an appropriate open source license and
    I have the right under that license to submit that work with
    modifications, whether created in whole or in part by me, under the
    Telodendria project license; or
  3. The contribution was provided directly to me by some other person
    who certified (1), (2), or (3), and I have not modified it.
  4. I understand and agree that this project and the contribution are
    made public and that a record of the contribution—including all
    personal information I submit with it—is maintained indefinitely
    and may be redistributed consistent with this project or the open
    source licenses involved.
  • I have read the Telodendria Project development certificate of
    origin, and I certify that I have permission to submit this patch
    under the conditions specified in it.
Implements everything in #26 : > - [x] **GET** /_telodendria/admin/tokens > - [x] **GET** /_telodendria/admin/tokens/[token] > - [x] **POST** /_telodendria/admin/tokens > - [x] **DELETE** /_telodendria/admin/tokens/[token] --- Please review the developer certificate of origin: 1. The contribution was created in whole or in part by me, and I have the right to submit it under the open source licenses of the Telodendria project; or 1. The contribution is based upon a previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the Telodendria project license; or 1. The contribution was provided directly to me by some other person who certified (1), (2), or (3), and I have not modified it. 1. I understand and agree that this project and the contribution are made public and that a record of the contribution—including all personal information I submit with it—is maintained indefinitely and may be redistributed consistent with this project or the open source licenses involved. - [X] I have read the Telodendria Project development certificate of origin, and I certify that I have permission to submit this patch under the conditions specified in it.
lda added 2 commits 2023-10-29 08:35:45 -05:00
lda added 1 commit 2023-10-31 05:29:22 -05:00
lda added 1 commit 2023-11-01 11:36:20 -05:00
lda added 1 commit 2023-11-01 11:52:02 -05:00
lda added 1 commit 2023-11-01 11:58:54 -05:00
lda added 1 commit 2023-11-01 12:04:38 -05:00
lda added 1 commit 2023-11-01 12:48:11 -05:00
jordan reviewed 2023-11-01 13:08:53 -05:00
src/include/RegToken.h
@ -109,0 +110,4 @@
* Returns a JSON object corresponding to a valid TokenInfo (see
* #26)
*/
extern HashMap * RegTokenJSON(RegTokenInfo *);
jordan commented 2023-11-01 13:08:53 -05:00
Owner
Copy Link

I wonder, is there any reason why RegTokenInfo can't be auto-generated with j2s? That would also generate the code to convert it to JSON automatically.

I wonder, is there any reason why `RegTokenInfo` can't be auto-generated with `j2s`? That would also generate the code to convert it to JSON automatically.
lda marked this conversation as resolved
lda added 1 commit 2023-11-01 15:12:38 -05:00
lda added 1 commit 2023-11-01 15:25:00 -05:00
lda changed title from WIP: Implement GET /_telodendria/admin/tokens to Implement GET /_telodendria/admin/tokens 2023-11-01 15:25:20 -05:00
jordan added this to the Telodendria v1.7.0-alpha4 project 2023-11-02 06:51:56 -05:00
lda added 1 commit 2023-11-02 07:03:28 -05:00
jordan reviewed 2023-11-04 13:57:20 -05:00
src/RegToken.c
@ -199,2 +201,4 @@
}
HashMap *
RegTokenJSON(RegTokenInfo * info)
jordan commented 2023-11-04 13:57:20 -05:00
Owner
Copy Link

I'm still a little confused about what's happening here.

You added a new TokenInfo schema, which looks like it works fine, but I wonder if there's still too much manual work happening here. I assumed we would just convert the existing RegTokenInfo structure into a schema, which we could then store in the database and send over the HTTP API.

Now, RegTokenInfo has db and ref properties that should be ignored when doing the JSON encoding/decoding; we might need to add this as an option to j2s.

Is there a reason that this wouldn't work? It might require some refactoring of the RegToken API, though. Is that the reason you did it this way?

I'm still a little confused about what's happening here. You added a new `TokenInfo` schema, which looks like it works fine, but I wonder if there's still too much manual work happening here. I assumed we would just convert the existing `RegTokenInfo` structure into a schema, which we could then store in the database and send over the HTTP API. Now, `RegTokenInfo` has `db` and `ref` properties that should be ignored when doing the JSON encoding/decoding; we might need to add this as an option to `j2s`. Is there a reason that this wouldn't work? It might require some refactoring of the `RegToken` API, though. Is that the reason you did it this way?
jordan commented 2023-11-04 13:59:49 -05:00
Owner
Copy Link

Either way, seeing lines like this

 tokinfo.name = info->name;
    tokinfo.created_on = info->created;
    tokinfo.expires_on = info->expires;

    tokinfo.uses = info->uses;
    tokinfo.used = info->used;

doesn't really make me feel good about what's happening here. It looks hard to maintain.

Either way, seeing lines like this ``` tokinfo.name = info->name; tokinfo.created_on = info->created; tokinfo.expires_on = info->expires; tokinfo.uses = info->uses; tokinfo.used = info->used; ``` doesn't really make me feel good about what's happening here. It looks hard to maintain.
lda commented 2023-11-04 16:36:58 -05:00
Author
Contributor
Copy Link

Honestly, I think a simpler solution would be to just do something like a big RegTokenInfo with all of the database stuff and all of the properties as a property(with type RegToken).

Honestly, I think a simpler solution would be to just do something like a big RegTokenInfo with all of the database stuff *and* all of the properties as a property(with type RegToken).
jordan commented 2023-11-06 15:30:59 -05:00
Owner
Copy Link

@lda Take a look at my pull request on your implement-26 branch and see if it is agreeable to you: lda/telodendria#1.

If it is, you should just be able to merge it and then I can merge your pull request into master.

@lda Take a look at my pull request on your `implement-26` branch and see if it is agreeable to you: https://git.telodendria.io/lda/telodendria/pulls/1. If it is, you should just be able to merge it and then I can merge your pull request into `master`.
jordan reviewed 2023-11-04 13:58:22 -05:00
src/RegToken.c
@ -201,0 +229,4 @@
/* The owner can be null if Telodendria created it.
* Since users can't contain a space, it is in this case set to
* "Telodendria Server". */
tokinfo.created_by = "Telodendria Server";
jordan commented 2023-11-04 13:58:22 -05:00
Owner
Copy Link

It should be fine to just send a NULL (or omit the key entirely) if there wasn't a specific user that created the token. I don't see why we need to hard-code Telodendria Server in this case.

It should be fine to just send a `NULL` (or omit the key entirely) if there wasn't a specific user that created the token. I don't see why we need to hard-code `Telodendria Server` in this case.
lda marked this conversation as resolved
jordan commented 2023-11-04 14:01:53 -05:00
Owner
Copy Link

This looks pretty good. I'm assuming you've tested it to verify that it works as intended?

This looks pretty good. I'm assuming you've tested it to verify that it works as intended?
lda added 2 commits 2023-11-04 16:45:58 -05:00
lda commented 2023-11-04 16:47:26 -05:00
Author
Contributor
Copy Link

This looks pretty good. I'm assuming you've tested it to verify that it works as intended?

I did some basic testing with the endpoints beforehand

> This looks pretty good. I'm assuming you've tested it to verify that it works as intended? I did some basic testing with the endpoints beforehand
jordan commented 2023-11-06 08:39:54 -05:00
Owner
Copy Link

I did some basic testing with the endpoints beforehand

Awesome. I'll trust your testing on this one because it looks like it should work fine.

> I did some basic testing with the endpoints beforehand Awesome. I'll trust your testing on this one because it looks like it should work fine.
jordan added spent time 2023-11-06 15:39:02 -05:00
2 hours 30 minutes
jordan commented 2023-11-06 15:52:55 -05:00
Owner
Copy Link

I should note that the documentation in #26 is slightly out of date. The endpoints should allow the caller to view and modify all of the properties in the RegTokenInfo structure, including the privileges.

I should note that the documentation in #26 is slightly out of date. The endpoints should allow the caller to view and modify all of the properties in the `RegTokenInfo` structure, including the privileges.
jordan added spent time 2023-11-08 09:37:58 -05:00
2 hours
jordan commented 2023-11-08 13:54:42 -05:00
Owner
Copy Link

Closing this because #43 is a significant improvement. Further discussion and development should take place there. If you find any issues with what is there, just open up a new pull request against the pull-37 branch that #43 is based on.

Closing this because #43 is a significant improvement. Further discussion and development should take place there. If you find any issues with what is there, just open up a new pull request against the `pull-37` branch that #43 is based on.
jordan closed this pull request 2023-11-08 13:54:42 -05:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug

Something is not working

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
question

More information is needed

  
wontfix

This won't be fixed

No Label
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Telodendria v1.7.0-alpha4
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Total Time Spent: 4 hours 30 minutes
jordan
4 hours 30 minutes
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Telodendria/Telodendria#37
No description provided.
Delete Branch "lda/telodendria:implement-26"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?