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

[ComputerCraft] Add new lua functions to Bank Terminal Peripheral #96

Merged
merged 5 commits into from
Aug 6, 2024

Conversation

banna12345bob
Copy link
Contributor

Adds four new lua functions to the Bank Terminal peripheral.

  • getAccounts()
  • getAccountLabel(String accountID)
  • getSubAccounts(String accountID)
  • getSubAccountLabel(String accountID, String authorizationID)

getAccounts()

Gets a list of UUIDs for all accounts.

Returns

  • list of UUIDs for all accounts.

getAccountLabel(String accountID)

Get the label of the specified account.
Throws an error if the account is not found.

Returns

  • string the name of the specified account.

getSubAccounts(String accountID)

Gives a list of UUIDs for any sub-accounts inside of specified account.
Only displays sub-accounts with authorisation type set to Anybody.

Returns

  • list of UUIDs for sub-accounts

getSubAccountLabel(String accountID, String authorizationID)

Returns the name for the specified sub-account inside of the specified account.
Authorisation type must be set to Anybody otherwise will throw an error.

Returns

  • string the name of the specified sub-account.

@banna12345bob banna12345bob changed the title [ComputerCraft] Add new lua functions for the Bank Terminal Peripheral [ComputerCraft] Add new lua functions to Bank Terminal Peripheral Jul 23, 2024
@techno-sam
Copy link
Member

I don't think it's a good idea to return a list of any sub accounts, because someone might collect IDs and wait till they're changed to Anybody

@banna12345bob
Copy link
Contributor Author

banna12345bob commented Jul 24, 2024

What function are you referring to? The getSubAccounts function only returns accounts set to Anybody.

@IThundxr IThundxr requested review from IThundxr and techno-sam July 26, 2024 13:47
@techno-sam
Copy link
Member

That's exactly my point. I don't want people to be able to get a list of Anybody accounts, because those should still have the security afforded by "I was careful not to give anyone I don't trust the UUID"

@banna12345bob
Copy link
Contributor Author

How do you intend users to get uuids of accounts that the owner wants them to access? Would it be preferable to put the uuids in the accounts/sub-accounts tab?
How would you expect users to remember all of their account uuids?

There is always a chance that malicious users read the uuid out of a already programmed computer thus rendering any security steps useless.

If you wish to contact me further about any issues with this PR I am .constantly on discord.

@techno-sam
Copy link
Member

Users are intended to get the UUIDs by using a computer to read them from an authorized card. Of course, malicious users could read the uuid from an unsecured computer, but, as long as the relevant server has some sort of claims mod, computers could be kept secure.

Other than enumerating sub accounts, I'm for merging this PR

Copy link
Member

@techno-sam techno-sam left a comment

Choose a reason for hiding this comment

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

Couple of nitpicks, but very good!

@banna12345bob
Copy link
Contributor Author

Documentation for new function.

isPlayerOwned(String accountID)

Whether a player owns the specified account or not.

Returns

  • bool true if account is player-owned, false if not.

Copy link
Member

@techno-sam techno-sam left a comment

Choose a reason for hiding this comment

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

lgtm

@techno-sam techno-sam merged commit 4d5f452 into Layers-of-Railways:1.20.1/dev Aug 6, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants