Improve types/studio typescript

[dawsontoth]

Once https://github.com/HarperFast/harperdb/pull/3045 lands and we released a new version (and I update the version referenced in this PR) we'll have greatly improved type safety for our GraphQL, Configs and JavaScript/TypeScript.

[kriszyp]

Do we want to define these as the first methods in a template? The new resource API (loadAsInstance = false) really minimizes the need for these allow* methods because the logic can more nicely be encapsulated into the REST methods themselves (in the old Resource API, allow* methods were more necessary because the record was loaded before the REST method was called), so I don't think we should suggest using them anymore.
It seems like it would be better to define the template:

export class Person extends tables.Person<PersonRecord> {
	static loadAsInstance = false;
	get(target: RequestTarget) {
		target.checkPermission = false; // this how we make reads public
		// any application logic
		return super.get(target);
	}

	put(target: RequestTarget, person: PersonRecord) {
		let user = this.getContext().user;
		// any custom permissions logic, or...
		// no modifications to target and super.put will perform the default permissions check
		// ... and we can do any application logic
		return super.put(target, person);
	}

	post(target: RequestTarget, data: any) {...
	delete(target: RequestTarget) {...
}

[dawsontoth]

@kriszyp I made the adjustments, but it feels quite a bit less natural because of a few things in our types from core.

1. getContext returns Context | SourceContext, so you'd need to sort out which you've received, and then do a .user or a .requestContext.user on it.
2. get may not receive a target (by the types, at least), or it may receive an Id (which I do see happen in the source code). If that happens, how will we set checkPermission to false? And would we need to do that on all the other spots where we are checking permission for the user too?
3. When testing locally, it seems like this.getContext().user is falling back to my local user even with basic auth not provided and no cookies provided.

[dawsontoth]

Ah, I think 3 is the desired behavior in dev mode, huh? TIL

[dawsontoth]

🤔 being able to static checkPermission = false would be nice, right? And if I add a getUser utility method too...

[dawsontoth]

Going to step away for lunch and a workout to ponder things a bit more (and some other stuff with AIs + auth)

[dawsontoth]

https://github.com/HarperFast/harperdb/pull/3046

[kriszyp]

getContext returns Context | SourceContext, so you'd need to sort out which you've received, and then do a .user or a .requestContext.user on it.

It should only return a Context, unless you are using the resource as a source, in which case you should not be checking user permissions since the resolved result could be used by many different requests/users, and only checking the permissions of the first request is nonsensical/wrong in almost all cases.

Id (which I do see happen in the source code)

Where? This can be called by harper on user resources?

If that happens, how will we set checkPermission to false?

> target = 1
1
> target.checkPermission = false
> !target.checkPermission
true

WFM! ;)

When testing locally, it seems like this.getContext().user is falling back to my local user even with basic auth not provided

That's what authentication.authorizeLocal does (and is supposed to do).

[kriszyp]

allow* methods were more necessary because the record was loaded before the REST method was called

To reiterate how much worse allow* methods are with the new resource API: Previously the allow* methods kinda made sense. We loaded the record before calling the REST method, and so the allow methods could access that preloaded record, make decisions about access based on that record. That was the primary use case for allow* methods, was to make decisions based on the record data, since broad table level access could be more controlled with role permissions. However, with the new resource API, the records are not preloaded. So if the allow* method needs to access the record (it usually does), it will need to do so explicitly. And if the REST methods needs to access record (it usually does), it loads it again. Which is really bad pattern to be encouraging. And if we can provide a single API (the REST methods) to give users what they need, which encourage a pattern of loading a record, doing any permission checks on that record, and then doing any app logic on the record, that's more efficient, cleaner, and simpler than the burden of learning two sets of methods/APIs.
I think we should just fully deprecate the allow* methods for v5.

[dawsontoth]

@kriszyp What do you think about, for v4, making it easier to skip the allow* methods with a boolean field on the resource? As per my PR.