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

feat(examples): auth pattern exploration #3406

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

Conversation

n0izn0iz
Copy link
Contributor

@n0izn0iz n0izn0iz commented Dec 25, 2024

WIP

Authentication pattern using auth token objects

  • p/demo/auth: core interfaces
  • r/demo/subacc: example auth provider that allows to create sub-accounts.
    To implement an auth provider you only need to create new auth.Token and auth.AuthenticateFn implementations
  • r/demo/sessions: another example auth provider that allows to whitelist addresses that can act as the mother account
  • r/demo/authreg: auth providers registry, allowing realms to manage ressource for any authentifiable entity
  • r/demo/authbanker is a basic example of allowing to manipulate coins with auth tokens

Signed-off-by: Norman <[email protected]>
@github-actions github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Dec 25, 2024
@Gno2D2
Copy link
Collaborator

Gno2D2 commented Dec 25, 2024

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):
  • IGNORE the bot requirements for this PR (force green CI check)
Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info)

☑️ Contributor Actions:
  1. Fix any issues flagged by automated checks.
  2. Follow the Contributor Checklist to ensure your PR is ready for review.
    • Add new tests, or document why they are unnecessary.
    • Provide clear examples/screenshots, if necessary.
    • Update documentation, if required.
    • Ensure no breaking changes, or include BREAKING CHANGE notes.
    • Link related issues/PRs, where applicable.
☑️ Reviewer Actions:
  1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.
📚 Resources:
Debug
Automated Checks
Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 The pull request was created from a fork (head branch repo: n0izn0iz/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Manual Checks
**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

  • Any user with comment edit permission

@n0izn0iz n0izn0iz changed the title feat: auth pattern exploration feat(examples): auth pattern exploration Dec 26, 2024
@moul moul self-requested a review December 26, 2024 13:30
// SendCoins sends `amount` coins from the vault identified by `atok` to the account identified by `to`.
//
// `to` can be an entity ID or an address.
func SendCoins(atok auth.Token, to string, amount int64) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you write a txtar or share a few maketx calls that you expect to use with this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is an integration test here

Copy link
Contributor Author

@n0izn0iz n0izn0iz Dec 27, 2024

Choose a reason for hiding this comment

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

Also added txtar here

panic("sent amount must be >= 0")
}

from := authreg.Authenticate(atok)
Copy link
Member

Choose a reason for hiding this comment

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

What prevents me from creating my own authenticator? I don't understand where you expect to whitelist the approved authenticators.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing, the pattern is meant to be extendable.

The registry namespaces the authenticators so you can't authenticate an entity from an other authenticator.

How would you exploit this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually there was a vulnerability in namespacing if you pass paths with .., maybe you were refering to that? I added a guard against that here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also if you want to whitelist providers somewhere, you can check the token's Source or statically import the providers and call their Authenticate function

Copy link
Member

Choose a reason for hiding this comment

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

I was considering creating a new authenticator, registering it, and then using it for universal acceptance.

I see two secure approaches: one is to implement a whitelist system on the registry itself, and the other is to establish a whitelist system within the contract that checks the authentication.

However, allowing anyone to create an authenticator and contracts to simply "verify if a token is valid" is definitely insecure.

@github-actions github-actions bot added the 📦 ⛰️ gno.land Issues or PRs gno.land package related label Dec 27, 2024
Copy link

codecov bot commented Dec 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@moul
Copy link
Member

moul commented Dec 28, 2024

I suggest you continue exploring this authentication pattern. Meanwhile, I will consider patterns that are more user-friendly with simple maketx commands, ideally without needing to create hashes or long strings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 ⛰️ gno.land Issues or PRs gno.land package related 🧾 package/realm Tag used for new Realms or Packages.
Projects
Status: 📥 Inbox
Status: Triage
Development

Successfully merging this pull request may close these issues.

3 participants